FYI -------- Original Message -------- Subject: [jgroups-dev] Incompatible change in 3.1.0 Date: Mon, 28 Nov 2011 17:36:49 +0100 From: Bela Ban <belaban(a)yahoo.com&gt; To: jg-users <javagroups-users(a)lists.sourceforge.net&gt;, jg-dev <javagroups-development(a)lists.sourceforge.net&gt; I'm working on an issue that requires a change in Message and RequestOptions: I want to change the 'flags' field from a byte to a short. I'd changethe follwoing methods and fields: - Message.OOB, DONT_BUNDLE, NO_FC, SCOPED, NO_RELIABILITY, NO_TOTAL_ORDER, NO_RELAY would become a short from a byte - void Message.setFlag(byte flag) --> void Message.setFlag(short flag); - void Message.clearFlag(byte flag) --> void Message.clearFlag(short flag) - boolean Message.isFlagSet(byte flag) --> boolean Message.isFlagSet(short flag) A typical use case such as: Message msg=new Message(...); msg.setFlag(Message.OOB); would *not* require any change at all. However, if you have: byte flags=Message.OOB; msg.setFlag(flags); This wouldn't work as 'flags' would have to be a short (or downcast to a byte). Also, in RequestOptions, the 'flags' field would have to be changed to a short (from a byte). RequestOptions.setFlags()/getFlags()/clearFlags() would be affected. Again, typical code like this: RequestOptions opts=new RequestOptions(ResponseMode.GET_ALL, 5000, false).setFlags(Message.OOB) would not have to be changed at all. What do people think ? I know this is an API change, although a minor one, and I wanted to see if anyone's code would break. I checked Infinispan (master) and this doesn't cause any code breakage. [1] https://issues.jboss.org/browse/JGRP-1250 _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

> > I think you can avoid breaking old clients here. Also, I don't think it's good to break such API in 3.1, should have been done in 3.0 :) No shit ? That's why I sent this email !$%#$ :-)

On Nov 29, 2011, at 10:50 AM, Bela Ban wrote: > > > On 11/29/11 10:20 AM, Galder Zamarreño wrote: >> Hmmm, why do you need to break API here? >> >> Couldn't you maintain the current API and add?: >> void Message.setMsgFlag(short flag) >> void Message.clearMsgFlag(short flag) >> boolean Message.isMsgFlagSet(short flag) > > > The problem here is that if you call Message.setFlag(byte flag) with > Message.OOB, you'll have to change your code anyway, namely narrow the > short to a byte. Right, that's assuming that you change to a short for Message.OOB. You could leave it as it is and have Message2.OOB which is a short.

If you're changing this, why not change it to an Enum to guard it from future changes? API-wise, flags are an Enum and impl-wise, the Enum can be used to generate any type of code for the wire. E.g., enum Flags { OOB((byte) 1), DONT_BUNDLE((byte) 2), … public Flags(byte code) { // … } } can then change to enum Flags { OOB((short) 1), DONT_BUNDLE((short) 2), … public Flags(short code) { // … } } or even enum Flags { OOB("OOB"), DONT_BUNDLE("Blah"), … public Flags(String code) { // … } } without ever breaking API. :) May as well future-proof it once and for all? - Manik On 28 Nov 2011, at 16:38, Bela Ban wrote: > FYI > > -------- Original Message -------- > Subject: [jgroups-dev] Incompatible change in 3.1.0 > Date: Mon, 28 Nov 2011 17:36:49 +0100 > From: Bela Ban&lt;belaban(a)yahoo.com&gt; > To: jg-users&lt;javagroups-users(a)lists.sourceforge.net&gt;, jg-dev > <javagroups-development(a)lists.sourceforge.net&gt; > > I'm working on an issue that requires a change in Message and > RequestOptions: I want to change the 'flags' field from a byte to a > short. I'd changethe follwoing methods and fields: > > - Message.OOB, DONT_BUNDLE, NO_FC, SCOPED, NO_RELIABILITY, > NO_TOTAL_ORDER, NO_RELAY would become a short from a byte > > - void Message.setFlag(byte flag) --> void Message.setFlag(short flag); > - void Message.clearFlag(byte flag) --> void Message.clearFlag(short flag) > - boolean Message.isFlagSet(byte flag) --> boolean > Message.isFlagSet(short flag) > > A typical use case such as: > > Message msg=new Message(...); > msg.setFlag(Message.OOB); > > would *not* require any change at all. > > However, if you have: > > byte flags=Message.OOB; > msg.setFlag(flags); > > This wouldn't work as 'flags' would have to be a short (or downcast to a > byte). > > Also, in RequestOptions, the 'flags' field would have to be changed to a > short (from a byte). RequestOptions.setFlags()/getFlags()/clearFlags() > would be affected. Again, typical code like this: > RequestOptions opts=new RequestOptions(ResponseMode.GET_ALL, 5000, > false).setFlags(Message.OOB) > > would not have to be changed at all. > > > What do people think ? I know this is an API change, although a minor > one, and I wanted to see if anyone's code would break. > > I checked Infinispan (master) and this doesn't cause any code breakage. > > > [1] https://issues.jboss.org/browse/JGRP-1250 > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev -- Manik Surtani manik(a)jboss.org twitter.com/maniksurtani Lead, Infinispan http://www.infinispan.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On 11/29/2011 11:28 AM, Bela Ban wrote: > Yes, I thought about this too, but then trashed the idea because enums > take up too much memory. Since an enum is a class, and an enum instance > is an instance (although a singleton / classloader instance), we have Why, you would just use the Enum when setting and getting, but otherwise just store your internal representation.

On 11/29/2011 11:59 AM, Bela Ban wrote: > > On 11/29/11 11:43 AM, Tristan Tarrant wrote: >> On 11/29/2011 11:28 AM, Bela Ban wrote: >>> Yes, I thought about this too, but then trashed the idea because enums >>> take up too much memory. Since an enum is a class, and an enum instance >>> is an instance (although a singleton / classloader instance), we have >> Why, you would just use the Enum when setting and getting, but otherwise >> just store your internal representation. > > Hmm, not a bad idea !  Although this way, you couldn't do > Message.setFlags((short)Message.OOB | Message.NO_FC), bitwise > operations, if OOB and NO_FC are enum instances... Why not use varargs here: Message.setFlags(Message.OOB, Message.NO_FC, ...);

> > Hmm, Message.setFlags() would look like this: > > public void setFlags(Flag flag) { >        flags |= (1<<  flag.ordinal()); > } > > > I do like the idea, although no using enums all the way and converting > them to/from shorts internally kind of defeats the purpose of enums... > > Actually, I don't think this breaks any API, unless people use the byte > or short values directly. > > Hmm, what do you do whith Message.getFlags() ? It returns a short. > Although this can be represented with an enum, multiple values can't, > e.g. 'OOB | NO_FC' cannot be represented as an enum... Maybe getFlags() > should be deprecated then ? >

> WDYT ? I'm willing to do this in 3.1, it's probably not worse than the > change from byte to short... > _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

> Hmm, not a bad idea ! Although this way, you couldn't do > Message.setFlags((short)Message.OOB | Message.NO_FC), bitwise > operations, if OOB and NO_FC are enum instances... Why not use varargs here: Message.setFlags(Message.OOB, Message.NO_FC, ...);

On 29 Nov 2011, at 10:43, Tristan Tarrant wrote: > On 11/29/2011 11:28 AM, Bela Ban wrote: >> Yes, I thought about this too, but then trashed the idea because enums >> take up too much memory. Since an enum is a class, and an enum instance >> is an instance (although a singleton / classloader instance), we have > Why, you would just use the Enum when setting and getting, but otherwise > just store your internal representation. Yes thats what I meant. This way the internal representation can change from version to version if needed, and still shield users from API change. Plus enums from an API standpoint is cleaner/nicer, and good enough a reason to do so.