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 <[email protected]> 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 <[email protected]> > 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 <[email protected]> > > 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 < > > [email protected]> > > > wrote: > > > > > > > Artem, what name do you plan to give to the BinaryInternalIdMapper? > > > > > > > > On Fri, Jan 15, 2016 at 1:52 AM, Artem Shutak <[email protected]> > > > > 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 < > > > > [email protected] > > > > > > > > > > > wrote: > > > > > > > > > > > On Thu, Jan 14, 2016 at 3:39 AM, Yakov Zhdanov < > > [email protected]> > > > > > > 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 < > > [email protected] > > > >: > > > > > > > > > > > > > > > I like the last one too. > > > > > > > > > > > > > > > > On Wed, Jan 13, 2016 at 7:54 AM, Artem Shutak < > > > > [email protected]> > > > > > > > > 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 -- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
