Enums with interface - how to do so generically?

Question

If I have a set of enums, and want to have them all implement an interface, is this the correct way to do generically?

Enum:

public enum MentalSkill implements SkillType {
    ACADEMICS,COMPUTER,CRAFTS,INVESTIGATION,MEDICINE,OCCULT,POLITICS,SCIENCE;
    private static final int UNTRAINED_PENALTY = -3;
    @Override
    public SkillType fromValue(String value) {
        return valueOf(value);
    }
    @Override
    public int getUntrainedPenalty() {
        return UNTRAINED_PENALTY;
    }
}

Interface:

public interface SkillType {

SkillType fromValue(String value);  
int getUntrainedPenalty();
}

Holding Class (where my suspcions lie that this is not quite right):

public class SkillsSet<T extends SkillType> {
    T t;
    Map<T,Integer> skills = new HashMap<>();
    public SkillsSet(String[] skills) {
        for (String string : skills) {
            addSkill(string,t.getUntrainedPenalty());
        }
    }
    private void addSkill(String skillString,Integer value) {
        skills.put((T) t.fromValue(skillString), 0);
    }
}

The issue comes in my T t which will obviously give a NPE as I don't instantiate my inferred type. The issue is that I can't, as it's an enum.

What's the Java way of saying 'Use this interface to access an enum method generically.'? Throwing in skills.put(T.fromValue(skillString), 0); doesn't work, which was my next guess.

I've looked over the tutorials (which is how I've gotten this far) and I couldn't seem to see how to get any further. How do I get this code to work?

Answer 1

One solution is through using reflection. I'm not sure if there are other solutions that might look cleaner though. The following does compile and run though.

public class SkillsSet<T extends SkillType> { 
    private Map<T,Integer> skills = new HashMap<>();

    // These don't have to be class members, they could be passed
    // around as parameters
    private T[] enumConstants;   // needed only for getEnumValue
    private Class<T> enumClass;  // needed only for getEnumValueAlternate

    public SkillsSet(String[] skills, Class<T> enumClass) {
        // Though all implementers of SkillType are currently enums, it is safer
        // to do some type checking before we do any reflection things
        enumConstants = enumClass.getEnumConstants();
        if (enumConstants == null)
            throw new IllegalArgumentException("enumClass is not an enum");

        for (String string : skills) {
            T t = getEnumValue(string)
            if (t == null) {
                // or use continue if you dont want to throw an exception
                throw new IllegalArgumentException();
            }
            this.skills.put(t, t.getPenalty());
        }
    }

    // These don't even need to be methods, but I separated them for clarity.
    // SuppressWarnings annotation is used since we checked types in the constructor
    @SuppressWarnings( "unchecked" )
    public T getEnumValue( String string ) {
        try {
            return (T) enumConstants[0].fromValue(string);
        }

        // If valueOf does not find a match, it throws IllegalArgumentExecption
        catch ( IllegalArgumentException e ) {
            return null;
        }
    }

    // An alternate version  of getEnumValue that does not require the 'fromValue'
    // method on the SkillType interface.
    @SuppressWarnings( "unchecked" )
    public T getEnumValueAlternate( String string ) {
        try {
            return (T) enumClass.getMethod("valueOf", String.class).invoke(null, string)
        }

        // Any reflection exceptions are treated as 'not found' on valueOf
        catch (Exception e) {
            return null; 
        }
    }


    ...
}

There are two different versions of getEnumValue. I'd recommend the version that uses the getEnumConstants method on the enumClass parameter. It also allows you to handle special cases in fromValue that might not typically match the enum's valueOf function.

Instantiating the above works with SkillsSet<MentalSkill> = new SkillsSet<MentalSkill>(new String[] {"COMPUTER", "CRAFTS"}, MentalSkill.class);

Answer 2

Your problem lies in SkillType - it should be generic:

public interface SkillType<T extends SkillType<T>> {

    T fromValue(String value);  

    int getUntrainedPenalty();
}

This is called a self referencing generic parameter.

Without this self reference, you could return an instance of a different class from the fromValue() method.

Here's how it would look:

public enum MentalSkill implements SkillType<MentalSkill> {
    public MentalSkill fromValue(...) {}
    ...
}

And

public class SkillsSet<T extends SkillType<T>> {
    ...
}

Once you make this change, it should all fall into place.

Answer 3

The issue comes in my T t which will obviously give a NPE as I don't instantiate my inferred type. The issue is that I can't, as it's an enum.

Actually, you couldn't even if it was a "regular" class! It is not possible to create instances of a parameter type within a generic class ... unless you pass in a Class<T> object as an explicit parameter.

In the case of an enum, you would either need to pass in a value of the enum, or pass in the Class<T> for the enum and use the static Enum<T>.valueOf(Class<T>, String) method to do a lookup.

Something like this would be needed ....

public class SkillsSet<T extends SkillType> {
    T t;
    Map<T,Integer> skills = new HashMap<>();
    public SkillsSet(String[] skills, T t) {
        this.t = t;
        for (String string : skills) {
            addSkill(string, t.getUntrainedPenalty());
        }
    }
    private void addSkill(String skillString, Integer value) {
        skills.put((T) t.fromValue(skillString), value);
    }
}

But the skills map doesn't make a lot of sense to me. Why would you use the "untrained penalty" for one T instance to initialise the skill value for all of the skills? It makes more sense if it is something more like this:

public class SkillsSet<T extends SkillType> { 
    private Map<T,Integer> skills = new HashMap<>();
    public SkillsSet(String[] skills, Class<T> enumClass) {
        for (String string : skills) {
            T t = Enum<T>.valueOf(enumClass, string);
            skills.put(t, t.getUntrainedPenalty());
        }
    }
    // Methods for accessing / updating the skills map information ...
}

You would instantiate a SkillSet like this:

SkillSet<MentalSkill> st = new SkillSet<>(new String[]{"COMPUTER", "OCCULT"},
                                          MentalSkill.class);

(Note: this is all uncompiled / untested code!!!)

Note that your fromValue method is unnecessary if you have a Class<T> object and you so that you can call Enum<T>.valueOf(...). And it is conceptually a bit odd. (You are using one instance of T as a factory object for other instances of T.)

Answer 4

Here

@Override
public SkillType fromValue(String value) {
    return valueOf(value);
}

the method valueOf(value) is a static one, it's not associated with an enum instance, which means you cannot call it generically. What you can do:

public class SkillsSet<T extends SkillType> {
    T t = MentalSkill.values()[0];
    Map<T,Integer> skills = new HashMap<>();

    public SkillsSet(String[] skills) {
        for (String string : skills) {
            addSkill(string,t.getUntrainedPenalty());
        }
    }

    private void addSkill(String skillString,Integer value) {
        skills.put((T) t.fromValue(skillString), 0);
    }
}

or more clearly

public class SkillsSet<T extends SkillType> {
    Map<T,Integer> skills = new HashMap<>();

    public SkillsSet(String[] skills) {
        for (String string : skills) {
            addSkill(string,t.getUntrainedPenalty());
        }
    }

    private void addSkill(String skillString,Integer value) {
        skills.put(MentalSkill.valueOf(skillString), 0);
    }
}

If you plan to have more enum types that implement SkillType then you need a factory over these types. However in this case, enum probably is not the best design decision.