John, Just to be clear. Not doing enum change correctly can cause problems with existing databases. That's a known problem with enum persistence in Java because enums by default using the name of the constant to represent the string of the constant. I was just cautioning anyone who wants to take that up to make sure they're doing the right thing. It doesn't require some huge (or even minor) framework/architectural change in CloudStack in order for enum changes to work with db.
--Alex > -----Original Message----- > From: John Burwell [mailto:jburw...@basho.com] > Sent: Friday, July 19, 2013 6:33 AM > To: dev@cloudstack.apache.org > Subject: Re: code formatting for enums > > Frank, > > I expect that renaming an enum value to be a minor change (i.e. once the > codebase compiles, the change is good). However, i can say I am surprised to > learn that such an operation could break the API and/or corrupt that > database. To my mind, this issue has nothing to do with developer choice. > Instead, our architecture needs to recognize that an enumerated value may > have multiple representations dependent on the context (e.g. CloudStack > HTTP API, AWS API, UI, code, database, etc), and provide mechanisms to > perform transformations between them when they need to differ. From a > persistence perspective, we need to should be keying off of a value that is > unlikely to change (e.g. an artificial ID). I would submit that Java > enumeration > value names, as code symbols, are subject to an unacceptable level of > change to be used for persistence. > > Thanks, > -John > > On Jul 18, 2013, at 8:49 PM, Frank Zhang <frank.zh...@citrix.com> wrote: > > > Yes it's brittle. I assume "law of least astonishment" here means > > reducing learning curve of developers and the changes they made should > > not surprise user. To achieve this the code must provide very flexibility to > developer. However, I am thinking of such kind of flexibility is really > needed, > isn't it? > > Currently "Convention over configuration" which sacrifices flexibility > > for simplicity is getting more and more popular. If we can state > > clearly to developers on constructing data structure from database tables > to API responses, we are achieving great simplicity and overall coding > convention. > > > > Sometimes, it's better off offering only choice to developer and don't make > them think. Anyway, a [DISCUSS] thread is necessary. > > > >> -----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 > >>>>>>>> > >>> > >
