Java Enum.valueOf() efficiency when value does not exist

Question

Which would you consider more efficient?

The use of 'WeekDay' is just an example:

public enum WeekDay {
    MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY;
}

Loop through and verify day string first:

public void parseString(String line) {
    String[] tokens = line.split();
    String day = tokens[1]; // day 'should' always be a weekday
    if (isValidWeekDay(day)) {
        WeekDay weekDay = WeekDay.valueOf(day); // won't throw exception
        ...
    } else {
        throw new InvalidWeekDayException(day); // subclass of RuntimeException
    }
}
private boolean isValidWeekDay(String day) {
    for (WeekDay weekDay : WeekDay.values()) {
        if(weekDay.toString().equals(day))
           return true;
    }
    return false;
}

Or since in 99.99% of cases, day will be correct:

public void parseString(String line) {
    String[] tokens = line.split();
    String day = tokens[1]; // day 'should' always be a weekday
    try {
        WeekDay weekDay = WeekDay.valueOf(day); // might throw exception
        ...
    } catch (IllegalArgumentException e) {
        throw new InvalidWeekDayException(day, e);
    }
}

---

Update:

To clarify, the input string will come from a client application, rather than a user. So in other words, it would be a bug to recieve a non workday in this example.

Answer 1

As has been commented, you will have to profile to find out for sure. Even in your own parsing approach, you can make it faster by returning the enum when you parse the list.

private WeekDay getValidWeekDay(String day) {
    for (WeekDay weekDay : WeekDay.values()) {
        if(weekDay.toString().equals(day))
           return weekDay;
    }
    return null;
}

Unless this is a time critical piece of an application, I wouldn't worry about it in either case and simply take the most readable approach. I think that would be using the WeekDay.valueOf() method.

If you would rather not have to deal with exceptions, then create a Map of your values within the enum and effectively do the equivalent of valueOf() from a lookup which returns null if it is not found.

public enum WeekDay {
    MONDAY, TUESDAY, WEDNESDAY, THURSDAY, FRIDAY;

    private static Map<String, WeekDay> valueMap;

    public static WeekDay getValue(String possibleName)
    {
        if (valueMap == null)
        {
            valueMap = new HashMap<String, WeekDay>();
            for(WeedDay day: values())
                valueMap.put(day.toString(), day);
        }
        return valueMap.get(possibleName);

    }
 }

This is effectively what the valueOf() method is doing anyway, except it throws the IllegalArgumentException when it is not found. This approach will simply return null, thus not generating the stacktrace.

Answer 2

What is the performance concern about the 2nd approach? Catching an exception like that costs almost nothing. Using exceptions for normal control flow is generally a bad idea from a design perspective, the days where this was a performance consideration are long gone. In a debugger, using exceptions as significant control operations will slow things down by a factor of about 10. But this gets optimized by the JIT and there is no measurable impact in production.

These numbers are based on experience with an evaluation I did of the zxing project, which uses exceptions for all sorts of flow control. When I first saw it, I was horrified. I still think it's not the best design, but I did quite a bit of testing and can say with a good bit of confidence that it had no real impact on performance. And this is an algorithm that was using exceptions all over the place for flow control. Your situation, where the exception will only get thrown in highly exceptional circumstances, is a non issue.

Edit: I've had a downvote or two on my answer, and I want to make sure that I'm super clear on what I'm saying: I do not think that it's a good idea to use exceptions for normal control flow. Just because performance is not a good argument for not using exceptions this way doesn't mean that there aren't other, perfectly valid reasons (such as readability, testability, extendability). In the case of the OP, the use of an exception is absolutely called for, and definitely wouldn't cause any sort of performance issue.

Answer 3

I know its an old post, but I believe following result will be still interesting. I run 10000000 tests to find an element in enum ENUM {FIRST, SECOND, THIRD, FOURTH, LAST} using JDK 1.8. The table below shows time required by simple loop and valueOf().

text     loop   valueOf  ratio
------------------------------
"FIRST"  121    65       186%
"LAST"   188    57       330%
"foo"    155    8958     1.7%

Conclusion - I wouldn't use valueOf() if I expect values not matching enum.

Answer 4

If your question is really about the efficiency of searching among 7 item you have already wasted too much time on it. Even the fastest search algorithms yield zero or negative benefits until N > 15 or so, other than the O(1) one.

Answer 5

Store the valid strings in a HashSet, and decide whether a string is a valid day or not based on Set.contains(...).

The set can be a static final Set, and you can wrap in an unmodifiable for good measure:

private static final Map<String> WEEKDAY_STRINGS;
static {
  HashSet<String> set = new HashSet();
  for (WeekDay d : WeekDay.values()) {
    set.add(d.toString());
  }
  WEEKDAY_STRINGS = Collections.unmodifiableSet(set);
}

Answer 6

The loop doesn't do anything that calling valueof doesn't, they have the same functionality : checking whether your string is valid enum. What do you think you gain from the first option ?

The second option is best:

 try {
     WeekDay weekDay = WeekDay.valueOf(day); // might throw exception
        ...
    } catch (IllegalArgumentException e) {
        throw new InvalidWeekDayException(day);
    }

Answer 7

Or you could create a lookup of enum values inside your enum when the class first loads(see static modifier) and validate using get() as shown below:

private String dayName;
private static final Map<String,Weekday> lookup = new HashMap<String, Weekday>();
static{
    for (Weekday day: values()){
        lookup.put(day.dayName, d);
    }
}
public static Weekday get(String _name){
    return lookup.get(_name);
}

Let me know if you need more details