tags:

views:

286

answers:

3

Hi I'm having trouble trying to generalize a function I've written for an specific enum:

public static enum InstrumentType {
    SPOT {
        public String toString() {
            return "MKP";
        }
    },
    VOLATILITY {
        public String toString() {
            return "VOL";
        }
    };

    public static InstrumentType parseXML(String value) {
        InstrumentType ret = InstrumentType.SPOT;

        for(InstrumentType instrumentType : values()) {
            if(instrumentType.toString().equalsIgnoreCase(value)) {
                ret = instrumentType;
                break;
            }
        }

        return ret;
    }
}

I wish to add a new parameter to the function that will represent any enum. I know I'm supposed to use templates but I can't use the function "values()" then inside the function code. Basicly what I want is a valueOf function that uses the toString() value I've defined.

Thanks in advance.

+15  A: 

Try a much cleaner way of writing your enum:

public static enum InstrumentType {

    SPOT("MKP"),
    VOLATILITY("VOL"),

    private final String name;

    InstrumentType(String name)
    {
     this.name = name;
    }

    public String toString()
    {
     return this.name;
    }

    public static InstrumentType getValue(String s)
    {
     for (InstrumentType t : InstrumentType.values())
     {
      if (t.toString().equals(s))
       return t;
     }
     return SOME_DEFAULT_VALUE;
    }
}

This also solves your problem of String -> Enum. It might be cleaner to just use the three-letter acronyms as the enum name, but in the case you will need to make the getValue() decision according to other parameters, this is the right way to go.

Yuval A
That is a neater way to overwrite the toString method, thanks.
eliocs
+1 We're naturally allergic to linear searches, so we tend not to realize that in small and medium enums it's never really going to cost that much.
Kevin Bourrillion
A: 

I believe that Enums can implement interfaces, so you could define one with a values() method and then use that as your parameter type.

But as the commenter said, it would probably be easier if you named your enums MKP, VOL etc

Phill Sacre
A: 

Update: Per James' comment below, the getEnumConstants() method on Class avoids all the messy reflection.

And as I should have remembered, Enum.valueOf(Class, String) already does exactly what this parseXML() method does, using the name() method rather than toString(). (My preferred approach would still be to define a special method and special "name" just for this purpose, though, rather than to use the enum's inherent name.)


If you pass in an instance of the enum class you think you're parsing, you can use reflection to access the values method generically, if you really want to -- though I wouldn't recommend it:

@SuppressWarnings("unchecked")
public static <T extends Enum<T>> T parseXML(Class<T> enumClass, String value) {
    try {
        Method valuesMethod = enumClass.getDeclaredMethod("values");
        T[] values = (T[]) valuesMethod.invoke(null);
        for (T t : values) {
            if (t.toString().equals(value)) {
                return t;
            }
        }
        throw new IllegalArgumentException("Bad value: " + value);
    } catch (NoSuchMethodException e) {
        // Should never happen: all enums have values()
        throw new IllegalStateException(e);
    } catch (InvocationTargetException e) {
        // Should never happen: values() can't be overridden, 
        // so there should be no way to blow it up
        throw new IllegalStateException(e);
    } catch (IllegalAccessException e) {
        // Should never happen: values() should always be public
        throw new IllegalStateException(e);
    }
}

One alternative would be to have all "parseable" enums register themselves with a "registry" class, e.g.

public class EnumRegistry {

    private static final Map<Class<?>, Map<String, Enum<?>>> registry 
      = new HashMap<Class<?>, Map<String, Enum<?>>>();

    /**
     * @param enumClass The class of the enum to register -- note that calling 
     * getClass() on the value isn't safe, b/c if an enum declares abstract
     * methods, each instance will be a different anonymous inner class
     */
    public static <T extends Enum<T>> void register(Class<T> enumClass, T t) {
        Map<String, Enum<?>> vals = registry.get(enumClass);
        if (vals == null) {
            vals = new HashMap<Class<?>, Map<String, Enum<?>>>();
            registry.put(enumClass, vals);
        }
        String strVal = t.toString();
        if (vals.containsKey(strVal)) {
            throw new IllegalArgumentException("Duplicate value: " + strVal);
        }
        vals.put(strVal, t);
    }

    @SuppressWarnings("unchecked")
    public static <T extends Enum<T>> T get(Class<T> enumClass, String s) {
        Map<String, Enum<?>> vals = registry.get(enumClass);
        if (vals == null) {
            return null;
        }
        return (T) vals.get(s);
    }
}

And to register:

public enum Foo {
  BAR, BAZ;

  private Foo() {
    EnumRegistry.register(Foo.class, this);
  }
}

Note: In any case, I'd suggest not overriding toString() but instead, as Phill Sacre suggests, introducing an interface that defines a different method specifically for this purpose, e.g. getName(). That way, among other things, you can be sure that only enums that are meant to be used with this XML mechanism of yours are involved, and you don't have to worry about the default implementation of toString() causing a screwup when somebody renames an enum instance. Plus, you can then use the same mechanism with other classes that are not actually Enum subclasses.

David Moles
Thats was what I was looking for, thank you very much for your indepth explaination.
eliocs
Or you could just use enumClass.getEnumConstants() and skip all that nasty reflection!
james
Much better, yes -- had no idea that was there. Though boy is it (and the fact that `Class` declares it) ever ugly -- going to do my best to forget I ever saw it. :)
David Moles
Yep, it sure is ugly... but it sure is useful!
james
reflection? total overkill...
Yuval A
Hey, the complaint was that he couldn't generically access the `values()` method.
David Moles