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 <ashu...@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 <dsetrak...@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 <ashu...@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 < >> dsetrak...@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 <ashu...@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 < >> > > dsetrak...@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 < >> ashu...@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 < >> > > > > dsetrak...@apache.org >> > > > > > > >> > > > > > wrote: >> > > > > > >> > > > > > > On Thu, Jan 14, 2016 at 3:39 AM, Yakov Zhdanov < >> > > yzhda...@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 < >> > > dsetrak...@apache.org >> > > > >: >> > > > > > > > >> > > > > > > > > I like the last one too. >> > > > > > > > > >> > > > > > > > > On Wed, Jan 13, 2016 at 7:54 AM, Artem Shutak < >> > > > > ashu...@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 -- >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > >> > > > >> > > >> > >> > >
