+1 to do this for any enum that was designed to be added to by the plugins.
I actually wrote a class for this before called Constant that did exactly this. The problem I had was that there was no way to guarantee compile time enumeration of all of the values like Enum did so it's possible for errors to be introduced but certainly we can follow a convention in this regard. Another way would be to use Spring to gather them and inject them into a holding class. --Alex > -----Original Message----- > From: Frank Zhang [mailto:frank.zh...@citrix.com] > Sent: Monday, July 22, 2013 5:45 PM > To: dev@cloudstack.apache.org > Subject: RE: [DESIGN] Why is enum a class... > > 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
