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>
*™*
