Better approach on handling unexpected string upon converting it to the enum value

Question

I have Grade enum and there is a static method to convert a string value to the particular enum value.

public enum Grade {
    None("N"),
    NotAccepted("-"),
    Accepted("+"),
    Uncertain("?");

    private final String gradeSymbol;

    Grade(String gradeSymbol) {
        this.gradeSymbol = gradeSymbol;
    }

    public String getGradeSymbol() {
        return gradeSymbol;
    }

    public static Grade fromGradeSymbol(final String gradeSymbol) {
        switch (gradeSymbol) {
            case "N":
                return None;

            case "-":
                return NotAccepted;

            case "+":
                return Accepted;

            case "?":
                return Uncertain;

            default:
                throw new IllegalArgumentException("Unexpected grade symbol: " + gradeSymbol);
        }
    }

    @Override
    public String toString() {
        return this.gradeSymbol;
    }
}

My question is, what is better way to handle unexpected grade symbol in the static fromGradeSymbol() method.

Another option I consider is the approach using Optional and returning Optional.empty() instead of throwing IllegalArgumentException.

Like this:

public static Optional<Grade> fromGradeSymbol(final String gradeSymbol) {
    switch (gradeSymbol) {
        case "N":
            return Optional.of(None);

        case "-":
            return Optional.of(NotAccepted);

        case "+":
            return Optional.of(Accepted);

        case "?":
            return Optional.of(Uncertain);

        default:
            return Optional.empty();
    }
}

My thoughts:

Throwing exception looks more logical, because it is being thrown in not expected case, when wrong value has been passed. But throwing exception makes it necessary to catch this exception anywhere where I convert string to the enum.

Use of Optional does not involve exceptions and makes cleaner code, without try-catch construct. But it somehow hides the unexpected input.

Sure, I can assume that every time when Optional does not contain a value we have an unexpected input. But it feels like code smell.

What do you think?

Answer 1

All enumerations have a default valueOf(String) method that throw an IllegalArgumentException so your first idea is coherent.

I would say that it depends (..). It depends of your use cases:

• If he conversion would be called form an unsafe (user/api) input, then there are chances that you receive an unexpected symbol. And using Optional will force the developer to handle that case more explicitely.
• If the convesrion is made from a safe input (code/config), then you can live with the exception.

In the book Effective Java from Joshua Bloch (Addison-Wesley Professional, 978-0134685991) there is a full chapter on that subject (in the 2nd edition):

Enum types have an automatically generated valueOf(String) method that translates a constant’s name into the constant itself.

[..]

The following code (with the type name changed appropriately) will do the trick for any enum, so long as each constant has a unique string representation:

// Implementing a fromString method on an enum type
private static final Map<String, Operation> stringToEnum
        = new HashMap<String, Operation>();
static { // Initialize map from constant name to enum constant
    for (Operation op : values())
        stringToEnum.put(op.toString(), op);
    }

// Returns Operation for string, or null if string is invalid
public static Operation fromString(String symbol) {
    return stringToEnum.get(symbol);
}

That chapter (and the whole book) is worth the read: CHAPTER 6 ENUMS AND ANNOTATIONS

Answer 2

Single Responsibility

My rule of thumb is that no extra methods or data should be added to an enum. If input has to be converted to an enum (or enum to output), a separate Function is created to handle the transformation. This way the enum does not get piggybacked with any additional responsibilities. The enum should be used for computation only.

Once you do that your problem becomes simple, as the responsibility of the unknown grade symbol is no longer responsibility of the enum. It becomes the responsibility of the component that should be responsible for incorrect input: the input handler. Now the correct action can be chosen from the specification that was made for the input handler. The Operation enum no longer has to worry about situations where it doesn't exist. It always exists.

This follows the single responsibility principle.

Thus:

public class SymbolToOperationConverter implements Function<String, Operation> {

    // Copy gervais.b's init routine.
    private static final Map<String, Operation> CONVERSION_MAP = ...;

    public Operation apply(String symbol) {
        Operation op = CONVERSION_MAP.get(symbol);
        if (op != null) {
            return op;
        } else {
            // My spec chooses to throw an excepotion.
            throw new InvalidInputException(...);
        }
    }
}

Useless Use Of Optional

Optionals are to Java programmers what cat is to Unix admins. More often than not they are completely unnecessary. Optionals were intended to be a way to communicate the possibility of null values in public interfaces in a code level (as opposed to documentation or annotations). The fromGradeSymbol method is completely internal to your application so you should know that it can return nulls and have unit tests for those use cases (that is, if you choose to not follow my advice above). There is literally nothing evil with returning a null value or testing for null with an if statement. The evil is not knowing if a third party library you use might return null or not. Optional was created to fight that evil.