ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Artem Shutak <ashu...@gridgain.com>
Subject Re: Binary ID mapper that uses a simple name of classes.
Date Fri, 22 Jan 2016 17:46:40 GMT
Dmitriy,

I think I got your point. I still have BinaryNameMapper and BinaryIdMapper
interfaces, but I have now by one implementation for each mapper with
properties as you wrote before.
- BinaryBaseNameMapper - an implementation of BinaryNameMapper that has
*setSimpleName(boolean isSImpleName)* property.
- BinaryBaseIdMapper - an implementation of BinaryBaseIdMapper that has
*setLowerCase(boolean usLowerCase)* property.

Thoughts?

-- Artem --

On Thu, Jan 21, 2016 at 1:35 PM, Artem Shutak <ashutak@gridgain.com> wrote:

> Dmitriy,
>
> Out-of-the-box Ignite wil have 2 IdMappers:
> - BinaryLowerCaseIdMapper - converts all of the characters in given
> string to lower case and then calls hashCode() for it.
> - BinaryStraightIdMapper - just calls hashCode() for given string
> (without converting to lower case).
>
> May be BinaryStraightIdMapper is not the best name for this mapper.
> Please, suggest another one if you have.
>
> I agree, that current design with BinaryNameMapper and BinaryIdMapper
> looks a little bit complex, but I don't see a better option to support all
> things described in my previous email. Dmitriy, If you have, please,
> describe your design in the ticket.
>
> Lets focus on names of mappers in this thread.
>
> Thanks,
> -- Artem --
>
> On Thu, Jan 21, 2016 at 6:10 AM, Dmitriy Setrakyan <dsetrakyan@apache.org>
> wrote:
>
>> Artem,
>>
>> My suggestion was that instead of having 3 id mappers that do almost the
>> same thing, have 1 ID mapper with additional configuration properties.
>>
>> BTW, I still don’t get what a straight ID mapper means.
>>
>> D.
>>
>> On Wed, Jan 20, 2016 at 9:45 AM, Artem Shutak <ashutak@gridgain.com>
>> wrote:
>>
>> > Dmitriy,
>> >
>> > BinaryStraightIdMapper - calculate hash code for given strings (do not
>> > change given string and call hashCode() for it). It's simplest
>> IdMapper. It
>> > would be nice if we will have it out-of-the-box.
>> >
>> > Could you please describe your approach in more details? Keep in mind,
>> that
>> > BinaryIdMapper is already a part of public API and, theoretically, a
>> user
>> > can write your own mapper to map class names to IDs (if he will have
>> > collisions for own classes) and custom mapper should know nothing about
>> > "simpleName" and "lowerCase" properties (for custom implementation).
>> >
>> > Lets me try to clarify current approach. At first, names classes and
>> fileds
>> > are processed by BinaryNameMapper and then these processed names pass to
>> > BinaryIdMapper for id calculation.
>> >
>> > Keep in mind the following:
>> > 1. It have to work with .Net.
>> > 2. it have to work without .Net and have to work with full name of
>> classes
>> > (to fix IGNITE-2191).
>> > 3. By some reasons, Ignite write *typeName* (for some types or for all,
>> not
>> > sure) at binary metadata.
>> >
>> > To support 1 and 2 we cannot always write full name or simple name at
>> > metadata. So, we have to have method like *String typeName(String
>> > fullClassName)* that returns processed class name (we have the one on
>> > BinaryNameMapper).
>> >
>> > I want to repeat that this design are consistent with .Net where we
>> already
>> > have NameMapper and IdMapper.
>> >
>> > Thnaks,
>> > -- Artem --
>> >
>> > On Wed, Jan 20, 2016 at 7:58 PM, Dmitriy Setrakyan <
>> dsetrakyan@apache.org>
>> > wrote:
>> >
>> > > Looks too busy.
>> > >
>> > > Why not just have one ID mapper with config properties, like
>> > > isSimpleName(), isLowerCase()?
>> > >
>> > > Also, what is a straight ID mapper?
>> > >
>> > > D.
>> > >
>> > > On Wed, Jan 20, 2016 at 4:01 AM, Artem Shutak <ashutak@gridgain.com>
>> > > wrote:
>> > >
>> > > > Dmitriy, thanks for reminding me about this thread.
>> > > >
>> > > > According to found issues in process of working on IGNITE-2191, it
>> was
>> > > > decided to add new entity BinaryNameMapper (in addition to
>> > > BinaryIdMapper)
>> > > > as a property of BinaryConfiguration. So, we will have the same
>> > > > configuration as we already have for .Net.
>> > > >
>> > > > So, we need to confirm names for BinaryNameMapper and
>> BinaryIdMapper. I
>> > > > propose the following:
>> > > >
>> > > > - BinarySimpleNameMapper / SimpleClassNameBinaryNameMapper /
>> > > > BinarySimpleClassNameMapper - returns simple name of class.
>> > > > - BinaryFullNameMapper / BinaryNoopNameMapper /
>> NoopBinaryNameMapper /
>> > > > BinaryOriginalNameMapper / StraightBinaryNameMapper - returns name
>> > > without
>> > > > changes.
>> > > > - BinaryLowerCaseIdMapper - it was BinaryInternalIdMapper.
>> > > > - BinaryStraightIdMapper - calculate hash code for given strings.
>> > > >
>> > > > I would chose BinarySimpleClassNameMapper, BinaryOriginalNameMapper,
>> > > > BinaryLowerCaseIdMapper and BinaryStraightIdMapper names.
>> > > >
>> > > > Thanks,
>> > > > -- Artem --
>> > > >
>> > > > On Wed, Jan 20, 2016 at 1:46 AM, Dmitriy Setrakyan <
>> > > dsetrakyan@apache.org>
>> > > > wrote:
>> > > >
>> > > > > Artem, what name do you plan to give to the
>> BinaryInternalIdMapper?
>> > > > >
>> > > > > On Fri, Jan 15, 2016 at 1:52 AM, Artem Shutak <
>> ashutak@gridgain.com>
>> > > > > wrote:
>> > > > >
>> > > > > > All Binary-related classes start from Binary. So, it's not
>> > > consistent.
>> > > > > >
>> > > > > > We should chose between *BinarySimpleNameIdMapper* and
>> > > > > > *BinarySimpleClassNameIdMapper*.
>> > > > > >
>> > > > > > Also, I'd like to move default *BinaryInternalIdMapper*
to
>> public
>> > > > package
>> > > > > > (that uses full class name) and rename him accordingly.
Any
>> > > objections?
>> > > > > >
>> > > > > > -- Artem --
>> > > > > >
>> > > > > > On Thu, Jan 14, 2016 at 11:21 PM, Dmitriy Setrakyan <
>> > > > > dsetrakyan@apache.org
>> > > > > > >
>> > > > > > wrote:
>> > > > > >
>> > > > > > > On Thu, Jan 14, 2016 at 3:39 AM, Yakov Zhdanov <
>> > > yzhdanov@apache.org>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > I would suggest "SimpleClassNameBinaryIdMapper"
>> > > > > > > >
>> > > > > > >
>> > > > > > > Is it consistent? Do we have other classes in the same
package
>> > that
>> > > > > start
>> > > > > > > with word Binary? If not, then I agree.
>> > > > > > >
>> > > > > > >
>> > > > > > > >
>> > > > > > > > --Yakov
>> > > > > > > >
>> > > > > > > > 2016-01-14 4:59 GMT+03:00 Dmitriy Setrakyan <
>> > > dsetrakyan@apache.org
>> > > > >:
>> > > > > > > >
>> > > > > > > > > I like the last one too.
>> > > > > > > > >
>> > > > > > > > > On Wed, Jan 13, 2016 at 7:54 AM, Artem Shutak
<
>> > > > > ashutak@gridgain.com>
>> > > > > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > Igniters,
>> > > > > > > > > >
>> > > > > > > > > > I'm working on
>> > > > https://issues.apache.org/jira/browse/IGNITE-2191
>> > > > > > > > (Binary
>> > > > > > > > > > marshaller: support user classes with
the same simple
>> name)
>> > > > bug.
>> > > > > > > > > >
>> > > > > > > > > > The fix expects new BinaryIdMapper that
uses a simple
>> name
>> > of
>> > > > > > > classes.
>> > > > > > > > It
>> > > > > > > > > > is required for supporting of platforms
(.Net. C++).
>> > > > > > > > > >
>> > > > > > > > > > I would be glade to hear suggestions
about good name for
>> > this
>> > > > > > mapper.
>> > > > > > > > > >
>> > > > > > > > > > I propose the following:
>> > > > > > > > > > - BinaryPlatformIdMapper
>> > > > > > > > > > - BinaryInteropIdMapper
>> > > > > > > > > > - SimpleNameBinaryIdMapper
>> > > > > > > > > > - BinarySimpleNameIdMapper
>> > > > > > > > > >
>> > > > > > > > > > I like the last one, but it has a big
length.
>> > > > > > > > > >
>> > > > > > > > > > Thanks,
>> > > > > > > > > > -- Artem --
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message