Agreed it's brittle. On the db side, it's not a big deal though. It just means when you change the enums, make sure the toString() method still retuns what's in the DB. I just wanted to caution that in case we're making changes.
On the API side, I generally think we shouldn't expose states directly to end users. That really does require a discussion thread. --Alex > -----Original Message----- > From: John Burwell [mailto:jburw...@basho.com] > Sent: Thursday, July 18, 2013 4:39 PM > To: dev@cloudstack.apache.org > Subject: Re: code formatting for enums > > Alex and Frank, > > In terms of conventions, the APIs exposed Java SDK and many other > common APIs following this convention. Hence, the reason for my > suggestion. > > The notion that changing an enum key can break the API and/or database > persistence feels a bit brittle. In particular, it doesn't conform to the > Law of > Least Surprise. It feels like we should have a transformation mechanism > from the API endpoint to an enum value, and employ foreign, artificial keys > to code tables in schema. This conversation has expanded a bit, and it seems > appropriate to open a new [DISCUSS] thread to delve into it further. Do y'all > agree? > > Thanks, > -John > > On Jul 18, 2013, at 7:19 PM, Frank Zhang <frank.zh...@citrix.com> wrote: > > > Those enums cannot be simply considered as internal data structures > where code convention applies to, they should be considered in API level. > > > > Most CloudStack API responses retrieve its fields from xxxVO classes > > which represent database table. In our example, enum State directly > > maps to UserVmResponse. state. Then the most important factor of name > convention is user experience where Running is more user friendly than > IS_RUNNING or whatever all capitalized sentence splitting by underscore. > > > > And any changes to those existing enum should be thought twice, as it > relates to API compatibility. > > > > Though we can introduce some mapping layer between internal enum and > > API response, I don't see any benefits if the only reason is to follow some > name convention. > > > >> -----Original Message----- > >> From: Alex Huang [mailto:alex.hu...@citrix.com] > >> Sent: Thursday, July 18, 2013 3:55 PM > >> To: dev@cloudstack.apache.org > >> Subject: RE: code formatting for enums > >> > >> Actually, that's more of a C/C++ coding convention. (Speaking of > >> which, please don't use "I" to start interfaces.) > >> > >> I prefer to have enums as follows > >> > >> Public class Vm { > >> enum State { > >> IsRunning, > >> Stopped, > >> } > >> } > >> > >> I generally like to write Vm.State.IsRunning in the code. It's readable > >> and > clear. > >> > >> As opposed to Vm.State.IS_RUNNING which is a little less readable. > >> > >> But the thing I've seen people do is just using IS_RUNNING or > >> State.IsRunning which often becomes confusing. I'm more against that > >> then all caps and underscore. > >> > >> My $.02. I will caution that any change to existing enums, we have > >> to think about how it maps to the database. If the VO object stores > >> the enum, you'll have to either upgrade the database or add methods > >> to the enum so that when storing it, it becomes the same. > >> > >> --Alex > >> > >>> -----Original Message----- > >>> From: John Burwell [mailto:jburw...@basho.com] > >>> Sent: Thursday, July 18, 2013 12:33 PM > >>> To: dev@cloudstack.apache.org > >>> Subject: Re: code formatting for enums > >>> > >>> All, > >>> > >>> Another thing I have noticed is that enum values are not capitalized. > >>> General coding convention is that enum values are declared in all > >>> caps using an underscore to separate words. I notice that our > >>> coding conventions are silent on enumerations. Any opposition to > >>> adding this rule to our coding conventions? > >>> > >>> Thanks, > >>> -John > >>> > >>> On Jul 17, 2013, at 12:24 PM, Alex Huang <alex.hu...@citrix.com> wrote: > >>> > >>>> That's because the first rule of auto-formatting is do no harm. > >>>> > >>>> The formatter is set not to screw with lines that are already > >>>> wrapped > >>> assuming the previous developer intended it that way. > >>>> > >>>> --Alex > >>>> > >>>>> -----Original Message----- > >>>>> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] > >>>>> Sent: Wednesday, July 17, 2013 8:23 AM > >>>>> To: dev > >>>>> Subject: Re: code formatting for enums > >>>>> > >>>>> thanks, > >>>>> it doesn't correct back to the one per line format, but at least > >>>>> it doesn't garble the enum when right anymore. > >>>>> > >>>>> > >>>>> > >>>>> On Wed, Jul 17, 2013 at 4:24 PM, Alex Huang > >>>>> <alex.hu...@citrix.com> > >>> wrote: > >>>>> > >>>>>> Windows->Preferences > >>>>>> Java->Formatter > >>>>>> Click on Edit in Active Profiles > >>>>>> Line Wrapping tab > >>>>>> Look for 'enum' declaration->Constants Select Wrap all elements, > >>>>>> every element on a new line in the "Line Wrapping policy:" drop > >>>>>> down > >>>>>> > >>>>>> --Alex > >>>>>> > >>>>>> > >>>>>>> -----Original Message----- > >>>>>>> From: Daan Hoogland [mailto:daan.hoogl...@gmail.com] > >>>>>>> Sent: Wednesday, July 17, 2013 6:22 AM > >>>>>>> To: dev > >>>>>>> Subject: code formatting for enums > >>>>>>> > >>>>>>> H, > >>>>>>> > >>>>>>> I am working on Networks with the eclipse.epf file loaded. Now > >>>>>>> the enum BroadcastDomainType gets saved as > >>>>>>> Native(null, null), Vlan("vlan", Integer.class), Vswitch("vs", > >>>>>>> String.class), LinkLocal(null, null), Vnet("vnet", > >>>>>> Long.class), Storage( > >>>>>>> "storage", Integer.class), Lswitch("lswitch", > >>>>>> String.class), Mido( > >>>>>>> "mido", String.class), Pvlan("pvlan", > >>>>>>> String.class), > >>>>>> UnDecided( > >>>>>>> null, null); > >>>>>>> instead of > >>>>>>> Native(null, null), > >>>>>>> Vlan("vlan", Integer.class), > >>>>>>> Vswitch("vs", String.class), > >>>>>>> LinkLocal(null, null), > >>>>>>> Vnet("vnet", Long.class), > >>>>>>> Storage("storage", Integer.class), > >>>>>>> Lswitch("lswitch", String.class), > >>>>>>> Mido("mido", String.class), > >>>>>>> Pvlan("pvlan", String.class), > >>>>>>> UnDecided(null, null); > >>>>>>> anybody know how to fix this? > >>>>>>> > >>>>>>> thanks, > >>>>>>> Daan > >>>>>> > >
