[java] Best practice to look up Java Enum

We have a REST API where clients can supply parameters representing values defined on the server in Java Enums.

So we can provide a descriptive error, we add this lookup method to each Enum. Seems like we're just copying code (bad). Is there a better practice?

public enum MyEnum {
    A, B, C, D;

    public static MyEnum lookup(String id) {
        try {
            return MyEnum.valueOf(id);
        } catch (IllegalArgumentException e) {
            throw new RuntimeException("Invalid value for my enum blah blah: " + id);
        }
    }
}

Update: The default error message provided by valueOf(..) would be No enum const class a.b.c.MyEnum.BadValue. I would like to provide a more descriptive error from the API.

Why do we have to write that 5 line code ?

public class EnumTest {
public enum MyEnum {
    A, B, C, D;
}

@Test
public void test() throws Exception {
    MyEnum.valueOf("A"); //gives you A
    //this throws ILlegalargument without having to do any lookup
    MyEnum.valueOf("RADD"); 
}
}

If you want the lookup to be case insensitive you can loop through the values making it a little more friendly:

 public enum MyEnum {
   A, B, C, D;

      public static MyEnum lookup(String id) {
        boolean found = false;
        for(MyEnum enum: values()){
           if(enum.toString().equalsIgnoreCase(id)) found = true;
        }  
        if(!found) throw new RuntimeException("Invalid value for my enum: " +id);
       }
}

The error message in IllegalArgumentException is already descriptive enough.

Your method makes a generic exception out of a specific one with the same message simply reworded. A developer would prefer the specific exception type and can handle the case appropriately instead of trying to handle RuntimeException.

If the intent is to make the message more user friendly, then references to values of enums is irrelevant to them anyway. Let the UI code determine what should be displayed to the user, and the UI developer would be better off with the IllegalArgumentException.

Looks like you have a bad practice here but not where you think.

Catching an IllegalArgumentException to rethrow another RuntimeException with a clearer message might look like a good idea but it is not. Because it means you care about messages in your exceptions.

If you care about messages in your exceptions, then it means that your user is somehow seeing your exceptions. This is bad.

If you want to provide an explicit error message to your user, you should check the validity of the enum value when parsing user input and send the appropriate error message in the response if user input is incorrect.

Something like:

// This code uses pure fantasy, you are warned!
class MyApi
{
    // Return the 24-hour from a 12-hour and AM/PM

    void getHour24(Request request, Response response)
    {
        // validate user input
        int nTime12 = 1;
        try
        {
            nTime12 = Integer.parseInt(request.getParam("hour12"));
            if( nTime12 <= 0 || nTime12 > 12 )
            {
                throw new NumberFormatException();
            }
        }
        catch( NumberFormatException e )
        {
            response.setCode(400); // Bad request
            response.setContent("time12 must be an integer between 1 and 12");
            return;
        }

        AMPM pm = null;
        try
        {
            pm = AMPM.lookup(request.getParam("pm"));
        }
        catch( IllegalArgumentException e )
        {
            response.setCode(400); // Bad request
            response.setContent("pm must be one of " + AMPM.values());
            return;
        }

        response.setCode(200);
        switch( pm )
        {
            case AM:
                response.setContent(nTime12);
                break;
            case PM:
                response.setContent(nTime12 + 12);
                break;
        }
        return;
    }
}

Guava also provides such function which will return an Optional if an enum cannot be found.

Enums.getIfPresent(MyEnum.class, id).toJavaUtil()
            .orElseThrow(()-> new RuntimeException("Invalid enum blah blah blah.....")))

We do all our enums like this when it comes to Rest/Json etc. It has the advantage that the error is human readable and also gives you the accepted value list. We are using a custom method MyEnum.fromString instead of MyEnum.valueOf, hope it helps.

public enum MyEnum {

    A, B, C, D;

    private static final Map<String, MyEnum> NAME_MAP = Stream.of(values())
            .collect(Collectors.toMap(MyEnum::toString, Function.identity()));

    public static MyEnum fromString(final String name) {
        MyEnum myEnum = NAME_MAP.get(name);
        if (null == myEnum) {
            throw new IllegalArgumentException(String.format("'%s' has no corresponding value. Accepted values: %s", name, Arrays.asList(values())));
        }
        return myEnum;
    }
}

so for example if you call

MyEnum value = MyEnum.fromString("X");

you'll get an IllegalArgumentException with the following message:

'X' has no corresponding value. Accepted values: [A, B, C, D]

you can change the IllegalArgumentException to a custom one.

You can use a static lookup map to avoid the exception and return a null, then throw as you'd like:

public enum Mammal {
    COW,
    MOUSE,
    OPOSSUM;

    private static Map<String, Mammal> lookup = 
            Arrays.stream(values())
                  .collect(Collectors.toMap(Enum::name, Function.identity()));

    public static Mammal getByName(String name) {
        return lookup.get(name);
    }
}

Apache Commons Lang 3 contais the class EnumUtils. If you aren't using Apache Commons in your projects, you're doing it wrong. You are reinventing the wheel!

There's a dozen of cool methods that we could use without throws an Exception. For example:

Gets the enum for the class, returning null if not found.

This method differs from Enum.valueOf in that it does not throw an exceptionfor an invalid enum name and performs case insensitive matching of the name.

EnumUtils.getEnumIgnoreCase(SeasonEnum.class, season);

update: As GreenTurtle correctly remarked, the following is wrong

I would just write

boolean result = Arrays.asList(FooEnum.values()).contains("Foo");

This is possibly less performant than catching a runtime exception, but makes for much cleaner code. Catching such exceptions is always a bad idea, since it is prone to misdiagnosis. What happens when the retrieval of the compared value itself causes an IllegalArgumentException ? This would then be treaten like a non matching value for the enumerator.