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
