cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Frank Zhang <Frank.Zh...@citrix.com>
Subject RE: code formatting for enums
Date Fri, 19 Jul 2013 18:58:33 GMT
Alex, John, I do think we should think carefully about changing the enum names.
First to me it's not a surprise that changing enum names may break API and database,
as in java enum name natively represent value. If we want to overcome this restriction
I would suggest directly using String so variable name won't effect final value in DB/API
anymore. 

Introducing some translation layer for enum would be a nightmare for maintenance.
Developers have to add lots of annotation on members of API class just for converting passed
in parameters to internal enum because java's enum convention is all capitalized. 

> -----Original Message-----
> From: Alex Huang [mailto:Alex.Huang@citrix.com]
> Sent: Friday, July 19, 2013 8:38 AM
> To: dev@cloudstack.apache.org
> Subject: RE: code formatting for enums
> 
> 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:jburwell@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.Zhang@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: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