.NET naming convention + RegisterSameJavaType mode

[Nikolay Izhikov-2]

Hello, Igniters.

Currently, BasicBinaryNameMapper[isSimpleName=false] assumes that type names equals both in Java and .Net.

        • Java type - ru.company.Model
        • .Net type - ‘ru.company.Model`

However, .net naming conventions [1] differs from java [2]
So, if a user follows both conventions then names will be

        • Java type - ru.company.Model
        • .Net type - 'Ru.Company.Model`

User can implement this behaviour in custom name mapper, but custom name mapper will disable RegisterSameJavaType mode.

To solve this issue I prepared a PR[3] and ticked [4].
PR introduces new BasicBinaryNameMapper#ForceJavaNamingConventions flag that enables mapping from .Net style to the java naming style.

WDYT?

[1] https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-namespaces
[2] https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html
[3] https://github.com/apache/ignite/pull/8795
[4] https://issues.apache.org/jira/browse/IGNITE-14169

[Nikolay Izhikov-2]

Hello, Pavel.

Thanks, for the feedback

> We can create a derived class so that existing _registerSameJavaType logic works.

For now, BinaryBasicNameMapper#GetTypeName is not virtual.
So we can’t override it.

And if we make GetTypeName virtual then any user can make extension of BinaryBasicNameMapper and use it.
I thought we want to avoid this.

Anyway, I’m OK with the built-in extension of BinaryBasicNameMapper.
Will do those changes.

> 13 февр. 2021 г., в 13:07, Pavel Tupitsyn <[hidden email]> написал(а):
>
> Nikolay,
>
> 1) What about the org./com. prefix that is present in all Java package names, e.g:
> Java: org.apache.ignite.foo.Bar
> .NET: Apache.Ignite.Foo.Bar
>
> I think we should handle this too.
>
> 2) Can we create a separate name mapper instead of adding a property to the existing one?
> We can create a derived class so that existing _registerSameJavaType logic works.
>
> On Fri, Feb 12, 2021 at 7:33 PM Nikolay Izhikov <[hidden email]> wrote:
> Hello, Igniters.
>
> Currently, BasicBinaryNameMapper[isSimpleName=false] assumes that type names equals both in Java and .Net.
>
>         • Java type - ru.company.Model
>         • .Net type - ‘ru.company.Model`
>
> However, .net naming conventions [1] differs from java [2]
> So, if a user follows both conventions then names will be
>
>         • Java type - ru.company.Model
>         • .Net type - 'Ru.Company.Model`
>
> User can implement this behaviour in custom name mapper, but custom name mapper will disable RegisterSameJavaType mode.
>
> To solve this issue I prepared a PR[3] and ticked [4].
> PR introduces new BasicBinaryNameMapper#ForceJavaNamingConventions flag that enables mapping from .Net style to the java naming style.
>
> WDYT?
>
> [1] https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-namespaces
> [2] https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html
> [3] https://github.com/apache/ignite/pull/8795
> [4] https://issues.apache.org/jira/browse/IGNITE-14169
>

[Pavel Tupitsyn]

> And if we make GetTypeName virtual then any user can make extension of
BinaryBasicNameMapper and use it.
> I thought we want to avoid this.

Yes, let's avoid this. Making those methods virtual makes everything even
harder to understand,
especially the logic behind RegisterSameJavaType: we don't want to enable
it for custom mappers.

I think the initial approach with an additional property was better.
In particular, because it is still a BinaryBasicNameMapper, and it
corresponds to the same mapper on Java side,
so we don't want to confuse the users with a different class name.

I propose to have the following two new properties in the
existing BinaryBasicNameMapper:
- bool NamespaceToLower
- string NamespacePrefix

Thoughts?

On Sun, Feb 14, 2021 at 12:03 PM Nikolay Izhikov <[hidden email]>
wrote:

> Hello, Pavel.
>
> Thanks, for the feedback
>
> > We can create a derived class so that existing _registerSameJavaType
> logic works.
>
> For now, BinaryBasicNameMapper#GetTypeName is not virtual.
> So we can’t override it.
>
> And if we make GetTypeName virtual then any user can make extension of
> BinaryBasicNameMapper and use it.
> I thought we want to avoid this.
>
> Anyway, I’m OK with the built-in extension of BinaryBasicNameMapper.
> Will do those changes.
>
> > 13 февр. 2021 г., в 13:07, Pavel Tupitsyn <[hidden email]>
> написал(а):
> >
> > Nikolay,
> >
> > 1) What about the org./com. prefix that is present in all Java package
> names, e.g:
> > Java: org.apache.ignite.foo.Bar
> > .NET: Apache.Ignite.Foo.Bar
> >
> > I think we should handle this too.
> >
> > 2) Can we create a separate name mapper instead of adding a property to
> the existing one?
> > We can create a derived class so that existing _registerSameJavaType
> logic works.
> >
> > On Fri, Feb 12, 2021 at 7:33 PM Nikolay Izhikov <[hidden email]>
> wrote:
> > Hello, Igniters.
> >
> > Currently, BasicBinaryNameMapper[isSimpleName=false] assumes that type
> names equals both in Java and .Net.
> >
> >         • Java type - ru.company.Model
> >         • .Net type - ‘ru.company.Model`
> >
> > However, .net naming conventions [1] differs from java [2]
> > So, if a user follows both conventions then names will be
> >
> >         • Java type - ru.company.Model
> >         • .Net type - 'Ru.Company.Model`
> >
> > User can implement this behaviour in custom name mapper, but custom name
> mapper will disable RegisterSameJavaType mode.
> >
> > To solve this issue I prepared a PR[3] and ticked [4].
> > PR introduces new BasicBinaryNameMapper#ForceJavaNamingConventions flag
> that enables mapping from .Net style to the java naming style.
> >
> > WDYT?
> >
> > [1]
> https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/names-of-namespaces
> > [2]
> https://www.oracle.com/java/technologies/javase/codeconventions-namingconventions.html
> > [3] https://github.com/apache/ignite/pull/8795
> > [4] https://issues.apache.org/jira/browse/IGNITE-14169
> >
>
>