cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Tutkowski <mike.tutkow...@solidfire.com>
Subject Re: [DESIGN] Why is enum a class...
Date Tue, 23 Jul 2013 15:43:15 GMT
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.Zhang@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.Huang@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.Huang@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.tutkowski@solidfire.com
o: 303.746.7302
Advancing the way the world uses the
cloud<http://solidfire.com/solution/overview/?video=play>
*™*

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message