BinaryObject API confuses users
Locked
BinaryObject API confuses users
 
|
Hello, Igniters.
We have confusing API in `BinaryObjectBuilder` class. The code below leads to the `ClassCastException` The cause is java method resolution rules [1] > There may be more than one such method, in which case the most specific one is chosen I suggest to deprecate `setField(String name, @Nullable BinaryObjectBuilder builder);` method and introduce `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` instead of it. What do you think? ``` public class BugTest extends GridCommonAbstractTest { @Test public void testBinaryObject() throws Exception { try (Ignite ignite = startGrid(0)) { BinaryObjectBuilder builder = ignite.binary().builder("testVal") .setField("name", "John Doe", String.class); builder.setField("name", builder.getField("name")); } } } ``` [1] https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 |
Re: BinaryObject API confuses users
|
|
Nikolay,
It seems to me, that the real cause of the issue is a contract: public <T> T getField(String name); Which can lead to class cast to any type. Consequently, a user gets runtime type checks instead of compile time checks. On the other hand, your proposal could be handy in practice, have nothing against it. пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > > Hello, Igniters. > > We have confusing API in `BinaryObjectBuilder` class. > > The code below leads to the `ClassCastException` > The cause is java method resolution rules [1] > > > There may be more than one such method, in which case the most specific one is chosen > > I suggest to deprecate `setField(String name, @Nullable BinaryObjectBuilder builder);` method and introduce > `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` instead of it. > > What do you think? > > ``` > public class BugTest extends GridCommonAbstractTest { > @Test public void testBinaryObject() throws Exception { > try (Ignite ignite = startGrid(0)) { > BinaryObjectBuilder builder = ignite.binary().builder("testVal") > .setField("name", "John Doe", String.class); > > builder.setField("name", builder.getField("name")); > } > } > } > ``` > > [1] https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 -- Best regards, Ivan Pavlukhin |
Re: BinaryObject API confuses users
|
|
In reply to this post by Nikolay Izhikov-2
Hello!
I can see where you are getting at, can we call it "setFieldNested" instead or something like that? setBinaryField is confusing because we're not passing anything especially binary, just a nested BinaryObjectBuilder. Regards, -- Ilya Kasnacheev пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > Hello, Igniters. > > We have confusing API in `BinaryObjectBuilder` class. > > The code below leads to the `ClassCastException` > The cause is java method resolution rules [1] > > > There may be more than one such method, in which case the most specific > one is chosen > > I suggest to deprecate `setField(String name, @Nullable > BinaryObjectBuilder builder);` method and introduce > `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` > instead of it. > > What do you think? > > ``` > public class BugTest extends GridCommonAbstractTest { > @Test public void testBinaryObject() throws Exception { > try (Ignite ignite = startGrid(0)) { > BinaryObjectBuilder builder = > ignite.binary().builder("testVal") > .setField("name", "John Doe", String.class); > > builder.setField("name", builder.getField("name")); > } > } > } > ``` > > [1] > https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 |
Re: BinaryObject API confuses users
|
|
Hello, Ilya.
I don’t get your point > We don’t passing … binary, just a … BinaryObjeсtBuilder. May be we should go with `setBinaryObjectBuilderField` or `setBinaryObjectField`? > 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email]> написал(а): > > Hello! > > I can see where you are getting at, can we call it "setFieldNested" instead > or something like that? > > setBinaryField is confusing because we're not passing anything especially > binary, just a nested BinaryObjectBuilder. > > Regards, > -- > Ilya Kasnacheev > > > пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > >> Hello, Igniters. >> >> We have confusing API in `BinaryObjectBuilder` class. >> >> The code below leads to the `ClassCastException` >> The cause is java method resolution rules [1] >> >>> There may be more than one such method, in which case the most specific >> one is chosen >> >> I suggest to deprecate `setField(String name, @Nullable >> BinaryObjectBuilder builder);` method and introduce >> `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` >> instead of it. >> >> What do you think? >> >> ``` >> public class BugTest extends GridCommonAbstractTest { >> @Test public void testBinaryObject() throws Exception { >> try (Ignite ignite = startGrid(0)) { >> BinaryObjectBuilder builder = >> ignite.binary().builder("testVal") >> .setField("name", "John Doe", String.class); >> >> builder.setField("name", builder.getField("name")); >> } >> } >> } >> ``` >> >> [1] >> https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 |
Re: BinaryObject API confuses users
|
|
Hello!
I think that repeating argument type name in method name is a code smell. Rather, we should describe how this method is different from other methods of its bunch. In this case, it is different since it allows you to create nested binary objects, i.e., ones with non-flat structure. Regards, -- Ilya Kasnacheev пт, 6 дек. 2019 г. в 13:45, Николай Ижиков <[hidden email]>: > Hello, Ilya. > > I don’t get your point > > > We don’t passing … binary, just a … BinaryObjeсtBuilder. > > May be we should go with `setBinaryObjectBuilderField` or > `setBinaryObjectField`? > > > > 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email]> > написал(а): > > > > Hello! > > > > I can see where you are getting at, can we call it "setFieldNested" > instead > > or something like that? > > > > setBinaryField is confusing because we're not passing anything especially > > binary, just a nested BinaryObjectBuilder. > > > > Regards, > > -- > > Ilya Kasnacheev > > > > > > пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > > > >> Hello, Igniters. > >> > >> We have confusing API in `BinaryObjectBuilder` class. > >> > >> The code below leads to the `ClassCastException` > >> The cause is java method resolution rules [1] > >> > >>> There may be more than one such method, in which case the most specific > >> one is chosen > >> > >> I suggest to deprecate `setField(String name, @Nullable > >> BinaryObjectBuilder builder);` method and introduce > >> `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` > >> instead of it. > >> > >> What do you think? > >> > >> ``` > >> public class BugTest extends GridCommonAbstractTest { > >> @Test public void testBinaryObject() throws Exception { > >> try (Ignite ignite = startGrid(0)) { > >> BinaryObjectBuilder builder = > >> ignite.binary().builder("testVal") > >> .setField("name", "John Doe", String.class); > >> > >> builder.setField("name", builder.getField("name")); > >> } > >> } > >> } > >> ``` > >> > >> [1] > >> > https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 > > |
Re: BinaryObject API confuses users
|
|
I wonder why signature is not
setField(String name, BinaryObject obj) пт, 6 дек. 2019 г. в 15:00, Ilya Kasnacheev <[hidden email]>: > Hello! > > I think that repeating argument type name in method name is a code smell. > Rather, we should describe how this method is different from other methods > of its bunch. > > In this case, it is different since it allows you to create nested binary > objects, i.e., ones with non-flat structure. > > Regards, > -- > Ilya Kasnacheev > > > пт, 6 дек. 2019 г. в 13:45, Николай Ижиков <[hidden email]>: > > > Hello, Ilya. > > > > I don’t get your point > > > > > We don’t passing … binary, just a … BinaryObjeсtBuilder. > > > > May be we should go with `setBinaryObjectBuilderField` or > > `setBinaryObjectField`? > > > > > > > 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email]> > > написал(а): > > > > > > Hello! > > > > > > I can see where you are getting at, can we call it "setFieldNested" > > instead > > > or something like that? > > > > > > setBinaryField is confusing because we're not passing anything > especially > > > binary, just a nested BinaryObjectBuilder. > > > > > > Regards, > > > -- > > > Ilya Kasnacheev > > > > > > > > > пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > > > > > >> Hello, Igniters. > > >> > > >> We have confusing API in `BinaryObjectBuilder` class. > > >> > > >> The code below leads to the `ClassCastException` > > >> The cause is java method resolution rules [1] > > >> > > >>> There may be more than one such method, in which case the most > specific > > >> one is chosen > > >> > > >> I suggest to deprecate `setField(String name, @Nullable > > >> BinaryObjectBuilder builder);` method and introduce > > >> `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` > > >> instead of it. > > >> > > >> What do you think? > > >> > > >> ``` > > >> public class BugTest extends GridCommonAbstractTest { > > >> @Test public void testBinaryObject() throws Exception { > > >> try (Ignite ignite = startGrid(0)) { > > >> BinaryObjectBuilder builder = > > >> ignite.binary().builder("testVal") > > >> .setField("name", "John Doe", String.class); > > >> > > >> builder.setField("name", builder.getField("name")); > > >> } > > >> } > > >> } > > >> ``` > > >> > > >> [1] > > >> > > > https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 > > > > > -- Best regards, Alexei Scherbakov |
Re: BinaryObject API confuses users
|
|
I’m too, Alex.
But, this signature leads to the same error as I mentioned. > 6 дек. 2019 г., в 15:53, Alexei Scherbakov <[hidden email]> написал(а): > > I wonder why signature is not > > setField(String name, BinaryObject obj) > > пт, 6 дек. 2019 г. в 15:00, Ilya Kasnacheev <[hidden email]>: > >> Hello! >> >> I think that repeating argument type name in method name is a code smell. >> Rather, we should describe how this method is different from other methods >> of its bunch. >> >> In this case, it is different since it allows you to create nested binary >> objects, i.e., ones with non-flat structure. >> >> Regards, >> -- >> Ilya Kasnacheev >> >> >> пт, 6 дек. 2019 г. в 13:45, Николай Ижиков <[hidden email]>: >> >>> Hello, Ilya. >>> >>> I don’t get your point >>> >>>> We don’t passing … binary, just a … BinaryObjeсtBuilder. >>> >>> May be we should go with `setBinaryObjectBuilderField` or >>> `setBinaryObjectField`? >>> >>> >>>> 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email]> >>> написал(а): >>>> >>>> Hello! >>>> >>>> I can see where you are getting at, can we call it "setFieldNested" >>> instead >>>> or something like that? >>>> >>>> setBinaryField is confusing because we're not passing anything >> especially >>>> binary, just a nested BinaryObjectBuilder. >>>> >>>> Regards, >>>> -- >>>> Ilya Kasnacheev >>>> >>>> >>>> пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: >>>> >>>>> Hello, Igniters. >>>>> >>>>> We have confusing API in `BinaryObjectBuilder` class. >>>>> >>>>> The code below leads to the `ClassCastException` >>>>> The cause is java method resolution rules [1] >>>>> >>>>>> There may be more than one such method, in which case the most >> specific >>>>> one is chosen >>>>> >>>>> I suggest to deprecate `setField(String name, @Nullable >>>>> BinaryObjectBuilder builder);` method and introduce >>>>> `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` >>>>> instead of it. >>>>> >>>>> What do you think? >>>>> >>>>> ``` >>>>> public class BugTest extends GridCommonAbstractTest { >>>>> @Test public void testBinaryObject() throws Exception { >>>>> try (Ignite ignite = startGrid(0)) { >>>>> BinaryObjectBuilder builder = >>>>> ignite.binary().builder("testVal") >>>>> .setField("name", "John Doe", String.class); >>>>> >>>>> builder.setField("name", builder.getField("name")); >>>>> } >>>>> } >>>>> } >>>>> ``` >>>>> >>>>> [1] >>>>> >>> >> https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 >>> >>> >> > > > -- > > Best regards, > Alexei Scherbakov |
Re: BinaryObject API confuses users
|
|
Looks like it's already possible to pass binary object as value using
BinaryObjectBuilder setField(String name, Object val); If it works we can remove a signature with BinaryObjectBuilder. The API is truly confusing. пт, 6 дек. 2019 г. в 15:55, Николай Ижиков <[hidden email]>: > I’m too, Alex. > > But, this signature leads to the same error as I mentioned. > > > > > 6 дек. 2019 г., в 15:53, Alexei Scherbakov <[hidden email]> > написал(а): > > > > I wonder why signature is not > > > > setField(String name, BinaryObject obj) > > > > пт, 6 дек. 2019 г. в 15:00, Ilya Kasnacheev <[hidden email]>: > > > >> Hello! > >> > >> I think that repeating argument type name in method name is a code > smell. > >> Rather, we should describe how this method is different from other > methods > >> of its bunch. > >> > >> In this case, it is different since it allows you to create nested > binary > >> objects, i.e., ones with non-flat structure. > >> > >> Regards, > >> -- > >> Ilya Kasnacheev > >> > >> > >> пт, 6 дек. 2019 г. в 13:45, Николай Ижиков <[hidden email]>: > >> > >>> Hello, Ilya. > >>> > >>> I don’t get your point > >>> > >>>> We don’t passing … binary, just a … BinaryObjeсtBuilder. > >>> > >>> May be we should go with `setBinaryObjectBuilderField` or > >>> `setBinaryObjectField`? > >>> > >>> > >>>> 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email]> > >>> написал(а): > >>>> > >>>> Hello! > >>>> > >>>> I can see where you are getting at, can we call it "setFieldNested" > >>> instead > >>>> or something like that? > >>>> > >>>> setBinaryField is confusing because we're not passing anything > >> especially > >>>> binary, just a nested BinaryObjectBuilder. > >>>> > >>>> Regards, > >>>> -- > >>>> Ilya Kasnacheev > >>>> > >>>> > >>>> пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > >>>> > >>>>> Hello, Igniters. > >>>>> > >>>>> We have confusing API in `BinaryObjectBuilder` class. > >>>>> > >>>>> The code below leads to the `ClassCastException` > >>>>> The cause is java method resolution rules [1] > >>>>> > >>>>>> There may be more than one such method, in which case the most > >> specific > >>>>> one is chosen > >>>>> > >>>>> I suggest to deprecate `setField(String name, @Nullable > >>>>> BinaryObjectBuilder builder);` method and introduce > >>>>> `setBinaryField(String name, @Nullable BinaryObjectBuilder builder);` > >>>>> instead of it. > >>>>> > >>>>> What do you think? > >>>>> > >>>>> ``` > >>>>> public class BugTest extends GridCommonAbstractTest { > >>>>> @Test public void testBinaryObject() throws Exception { > >>>>> try (Ignite ignite = startGrid(0)) { > >>>>> BinaryObjectBuilder builder = > >>>>> ignite.binary().builder("testVal") > >>>>> .setField("name", "John Doe", String.class); > >>>>> > >>>>> builder.setField("name", builder.getField("name")); > >>>>> } > >>>>> } > >>>>> } > >>>>> ``` > >>>>> > >>>>> [1] > >>>>> > >>> > >> > https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 > >>> > >>> > >> > > > > > > -- > > > > Best regards, > > Alexei Scherbakov > > -- Best regards, Alexei Scherbakov |
Re: BinaryObject API confuses users
|
|
Hello!
I think we should check that setField happily accepts both BinaryObject and BinaryObjectBuilder, and then deprecate this method. Regards, -- Ilya Kasnacheev пт, 6 дек. 2019 г. в 16:02, Alexei Scherbakov <[hidden email] >: > Looks like it's already possible to pass binary object as value using > BinaryObjectBuilder setField(String name, Object val); > > If it works we can remove a signature with BinaryObjectBuilder. > > The API is truly confusing. > > пт, 6 дек. 2019 г. в 15:55, Николай Ижиков <[hidden email]>: > > > I’m too, Alex. > > > > But, this signature leads to the same error as I mentioned. > > > > > > > > > 6 дек. 2019 г., в 15:53, Alexei Scherbakov < > [hidden email]> > > написал(а): > > > > > > I wonder why signature is not > > > > > > setField(String name, BinaryObject obj) > > > > > > пт, 6 дек. 2019 г. в 15:00, Ilya Kasnacheev <[hidden email] > >: > > > > > >> Hello! > > >> > > >> I think that repeating argument type name in method name is a code > > smell. > > >> Rather, we should describe how this method is different from other > > methods > > >> of its bunch. > > >> > > >> In this case, it is different since it allows you to create nested > > binary > > >> objects, i.e., ones with non-flat structure. > > >> > > >> Regards, > > >> -- > > >> Ilya Kasnacheev > > >> > > >> > > >> пт, 6 дек. 2019 г. в 13:45, Николай Ижиков <[hidden email]>: > > >> > > >>> Hello, Ilya. > > >>> > > >>> I don’t get your point > > >>> > > >>>> We don’t passing … binary, just a … BinaryObjeсtBuilder. > > >>> > > >>> May be we should go with `setBinaryObjectBuilderField` or > > >>> `setBinaryObjectField`? > > >>> > > >>> > > >>>> 6 дек. 2019 г., в 13:38, Ilya Kasnacheev <[hidden email] > > > > >>> написал(а): > > >>>> > > >>>> Hello! > > >>>> > > >>>> I can see where you are getting at, can we call it "setFieldNested" > > >>> instead > > >>>> or something like that? > > >>>> > > >>>> setBinaryField is confusing because we're not passing anything > > >> especially > > >>>> binary, just a nested BinaryObjectBuilder. > > >>>> > > >>>> Regards, > > >>>> -- > > >>>> Ilya Kasnacheev > > >>>> > > >>>> > > >>>> пт, 6 дек. 2019 г. в 13:17, Николай Ижиков <[hidden email]>: > > >>>> > > >>>>> Hello, Igniters. > > >>>>> > > >>>>> We have confusing API in `BinaryObjectBuilder` class. > > >>>>> > > >>>>> The code below leads to the `ClassCastException` > > >>>>> The cause is java method resolution rules [1] > > >>>>> > > >>>>>> There may be more than one such method, in which case the most > > >> specific > > >>>>> one is chosen > > >>>>> > > >>>>> I suggest to deprecate `setField(String name, @Nullable > > >>>>> BinaryObjectBuilder builder);` method and introduce > > >>>>> `setBinaryField(String name, @Nullable BinaryObjectBuilder > builder);` > > >>>>> instead of it. > > >>>>> > > >>>>> What do you think? > > >>>>> > > >>>>> ``` > > >>>>> public class BugTest extends GridCommonAbstractTest { > > >>>>> @Test public void testBinaryObject() throws Exception { > > >>>>> try (Ignite ignite = startGrid(0)) { > > >>>>> BinaryObjectBuilder builder = > > >>>>> ignite.binary().builder("testVal") > > >>>>> .setField("name", "John Doe", String.class); > > >>>>> > > >>>>> builder.setField("name", builder.getField("name")); > > >>>>> } > > >>>>> } > > >>>>> } > > >>>>> ``` > > >>>>> > > >>>>> [1] > > >>>>> > > >>> > > >> > > > https://docs.oracle.com/javase/specs/jls/se11/html/jls-15.html#jls-15.12.2 > > >>> > > >>> > > >> > > > > > > > > > -- > > > > > > Best regards, > > > Alexei Scherbakov > > > > > > -- > > Best regards, > Alexei Scherbakov > |
«
Return to Apache Ignite Developers - Legacy Mail Archive
|
1 view|%1 views
| Free forum by Nabble | Edit this page |