cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Huang <Alex.Hu...@citrix.com>
Subject RE: code formatting for enums
Date Thu, 18 Jul 2013 23:44:29 GMT
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:jburwell@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.Zhang@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.Huang@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:jburwell@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.huang@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.hoogland@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.Huang@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.hoogland@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
> >>>>>>
> >


Mime
View raw message