Emm, it's not intended as a pun, but now I learned this pun :)

> -----Original Message-----
> From: Mike Tutkowski [mailto:mike.tutkow...@solidfire.com]
> Sent: Tuesday, July 23, 2013 8:43 AM
> To: dev@cloudstack.apache.org
> Subject: Re: [DESIGN] Why is enum a class...
> 
> The first thing I noticed about Frank's comment is that he started it with
> "Frankly speaking." (Frank/Frankly) Probably not intended as a pun, but funny
> nonetheless. :)
> 
> 
> On Mon, Jul 22, 2013 at 6:45 PM, Frank Zhang <frank.zh...@citrix.com>
> wrote:
> 
> > Frankly speaking, if we are going to change enum, I would suggest not
> > using enmu anymore, instead, defining our own class like:
> >
> > public class VmType {
> >         private static Map<String, VmType > types =
> > Collections.synchronizedMap(new HashMap<String, VmType >());
> >         private final String typeName;
> >
> >         public VmType (String typeName) {
> >                 this.typeName = typeName;
> >                 types.put(typeName, this);
> >         }
> >
> >         public static VmType valueOf(String typeName) {
> >                 VmType type = types.get(typeName);
> >                 if (type == null) {
> >                         throw new IllegalArgumentException("VmType type: "
> > + typeName + " was not registered by any VmType");
> >                 }
> >                 return type;
> >         }
> >
> >         @Override
> >         public String toString() {
> >                 return typeName;
> >         }
> >
> >         @Override
> >         public boolean equals(Object t) {
> >                 if (t == null || !(t instanceof VmType)) {
> >                         return false;
> >                 }
> >
> >                 VmType type = (VmType)t;
> >                 return type.toString().equals(typeName);
> >         }
> >
> >         @Override
> >         public int hashCode() {
> >                 return typeName.hashCode();
> >         }
> >
> >         public static Set<String> getAllTypeNames() {
> >             return types.keySet();
> >         }
> > }
> >
> > The only benefit of enum I see is you can use "==" instead of "equals()"
> > when comparing variables. However, it makes your code tight, every
> > time adding a new type you have to modify the enum.
> >
> > By above method, when a new plugin wants to extend a new VmType, it
> > simply
> > does:
> >
> > class MagicVmManagerImpl {
> >         public static final VmType type = new VmType("MagicVm"); }
> >
> >
> > > -----Original Message-----
> > > From: Alex Huang [mailto:alex.hu...@citrix.com]
> > > Sent: Monday, July 22, 2013 5:23 PM
> > > To: dev@cloudstack.apache.org
> > > Subject: RE: [DESIGN] Why is enum a class...
> > >
> > > BTW, this code already shows a bug that stems from the static method
> > usage.
> > > It says ElasticIpVm is not a system vm (which I don't believe is true).
> >  Probably
> > > because whoever added elastic ip vm didn't see the static method and so
> > didn't
> > > add the vm into the method.
> > >
> > > --Alex
> > >
> > > > -----Original Message-----
> > > > From: Alex Huang [mailto:alex.hu...@citrix.com]
> > > > Sent: Monday, July 22, 2013 5:14 PM
> > > > To: dev@cloudstack.apache.org
> > > > Subject: [DESIGN] Why is enum a class...
> > > >
> > > > I just went over this code and thought it was related and might be
> > > > interested to other developers.
> > > >
> > > > What's the difference between declaring a enum like this
> > > >
> > > >     public enum Type {
> > > >         User,
> > > >         DomainRouter,
> > > >         ConsoleProxy,
> > > >         SecondaryStorageVm,
> > > >         ElasticIpVm,
> > > >         ElasticLoadBalancerVm,
> > > >         InternalLoadBalancerVm,
> > > >
> > > >         /*
> > > >          * UserBareMetal is only used for selecting
> > > > VirtualMachineGuru, there is no
> > > >          * VM with this type. UserBareMetal should treat exactly as
> > User.
> > > >          */
> > > >         UserBareMetal;
> > > >
> > > >         public static boolean isSystemVM(VirtualMachine.Type vmtype) {
> > > >             if (DomainRouter.equals(vmtype)
> > > >                     || ConsoleProxy.equals(vmtype)
> > > >                     || SecondaryStorageVm.equals(vmtype) ||
> > > > InternalLoadBalancerVm.equals(vmtype)) {
> > > >                 return true;
> > > >             }
> > > >             return false;
> > > >         }
> > > >     }
> > > >
> > > > Vs
> > > >
> > > >     public enum Type {
> > > >         User(false),
> > > >         DomainRouter(true),
> > > >         ConsoleProxy(true),
> > > >         SecondaryStorageVm(true),
> > > >         ElasticIpVm(true),
> > > >         ElasticLoadBalancerVm(true),
> > > >         InternalLoadBalancerVm(true),
> > > >
> > > >         /*
> > > >          * UserBareMetal is only used for selecting
> > > > VirtualMachineGuru, there is no
> > > >          * VM with this type. UserBareMetal should treat exactly as
> > User.
> > > >          */
> > > >         UserBareMetal(false);
> > > >
> > > >        private boolean _isSystemVm;
> > > >
> > > >        private Type(Boolean isSystemVm) {
> > > >            _isSystemVm = isSystemVm;
> > > >        }
> > > >
> > > >         public boolean isSystemVM() {
> > > >            return _isSystemVm;
> > > >         }
> > > >     }
> > > >
> > > > The second declaration is more self-descriptive:
> > > >
> > > > - It tells developer when they add more to this enum, they have to
> > > > specify whether it is a system vm.  They may have missed the static
> > > > method in the first declaration and causes failures later.
> > > > - It tells developers using the enum that there's a property that
> > > > let's them know it is a system vm so they can do discovery using a
> > > > context-sensitive editor like Eclipse.
> > > >
> > > > This is the reason why enum is not a simple constant declaration in
> > > > Java (vs C/C++).
> > > > --Alex
> >
> 
> 
> 
> --
> *Mike Tutkowski*
> *Senior CloudStack Developer, SolidFire Inc.*
> e: mike.tutkow...@solidfire.com
> o: 303.746.7302
> Advancing the way the world uses the
> cloud<http://solidfire.com/solution/overview/?video=play>
> *(tm)*

Reply via email to