Java enum reverse look-up best practice

Question

I saw it suggested on a blog that the following was a reasonable way to do a "reverse-lookup" using the getCode(int) in a Java enum:

public enum Status {
    WAITING(0),
    READY(1),
    SKIPPED(-1),
    COMPLETED(5);

    private static final Map<Integer,Status> lookup 
            = new HashMap<Integer,Status>();

    static {
        for(Status s : EnumSet.allOf(Status.class))
            lookup.put(s.getCode(), s);
    }

    private int code;

    private Status(int code) {
        this.code = code;
    }

    public int getCode() { return code; }

    public static Status get(int code) { 
        return lookup.get(code); 
    }
}

To me, the static map and the static initializer both look like a bad idea, and my first thought would be to code the lookup as so:

public enum Status {
    WAITING(0),
    READY(1),
    SKIPPED(-1),
    COMPLETED(5);

    private int code;

    private Status(int code) {
        this.code = code;
    }

    public int getCode() { return code; }

    public static Status get(int code) { 
        for(Status s : values()) {
            if(s.code == code) return s;
        }
        return null;
    }
}

Are there any obvious problems with either method, and is there a recommended way to implement this kind of lookup?

Btw, for you Map building loop you could have done `for(Status s : values()) lookup.put(s.code, s);` — Peter Lawrey, Mar 15 '11 at 19:40
Is there something wrong with using `Enum.valueOf()`? Are you unable to store Strings? — Jonathan, Mar 15 '11 at 20:06
@Jonathan Quite often you need to produce enumerations from binary or number input. So I guess there is nothing *wrong* with `Enum.valueOf()` (mind capitalization though) but quite often you've just got a byte or a number to start with. And please: if a string is not needed, leave it out, lookup "stringly typed coding horror" if you want to know why. Basically you should ask yourself continiously: when I receive a string, do I know what is in it? It contains way more state than a integer or, indeed, an enum and *state increase is bad*. — Maarten Bodewes, Dec 21 '18 at 22:46
Have a look at https://stackoverflow.com/questions/28762438/how-to-reverse-enum — bohemian, Jan 09 '19 at 17:47

score 31 · Answer 1 · edited Jul 30 '19 at 13:48

31

Maps.uniqueIndex from Google's Guava is quite handy for building lookup maps.

Update: Here is an example using Maps.uniqueIndex with Java 8:

public enum MyEnum {
    A(0), B(1), C(2);

    private static final Map<Integer, MyEnum> LOOKUP = Maps.uniqueIndex(
                Arrays.asList(MyEnum.values()),
                MyEnum::getStatus
    );    

    private final int status;

    MyEnum(int status) {
        this.status = status;
    }

    public int getStatus() {
        return status;
    }

    @Nullable
    public static MyEnum fromStatus(int status) {
        return LOOKUP.get(status);
    }
}

edited Jul 30 '19 at 13:48

Michael

41,989
11
82
128

answered Mar 15 '11 at 18:33

2

This is a fine answer and it does get rid of the `static` class initializer. Does anyone know if it has any other advantages over a specific `Map` from Java (just getting rid of a static initializer for a field specific initializer is not enough reason for me to include a library into my classpath, even if that library is Guava). – Maarten Bodewes Dec 21 '18 at 22:24
In my opinion, this is a hacky solution that adds an innecessary dependency on an external library. A nicer, more elegant solution can be found here https://stackoverflow.com/questions/28762438/how-to-reverse-enum – bohemian Jan 09 '19 at 17:46
6

Of course with streams you don't need Guava: `LOOKUP = stream(values()).collect(toMap(MyEnum::getStatus, x -> x));`. – Gene May 29 '19 at 17:01
1

This works for me: LOOKUP = Arrays.stream(values()) .collect(Collectors.toMap(MyEnum::getStatus, Function.identity())); – Kjeld Oct 05 '21 at 13:02

score 20 · Answer 2 · answered Mar 15 '11 at 18:34

20

Though it has higher overhead, the static map is nice because it offers constant-time lookup by code. Your implementation's lookup time increases linearly with the number of elements in the enum. For small enums, this simply will not contribute significantly.

One issue with both implementations (and, arguably, with Java enums in general) is that there's really a hidden extra value that a Status can take on: null. Depending on the rules of the business logic, it may make sense to return an actual enum value, or throw an Exception, when the lookup "fails."

answered Mar 15 '11 at 18:34

Matt Ball

354,903
100
647
710

2

@Matt, I believe both ways are constant time lookup because there is a constant number of items in the Map. – jjnguy Mar 15 '11 at 18:36
2

@jjnguy: nope. Doing a lookup through the array of values requires inspecting every element in the list. If you increase the number of elements in the enum, you increase the number of elements that might be inspected for a reverse lookup. – Matt Ball Mar 15 '11 at 18:38
@Matt, right...but both approaches are `O(1)`. The one that does the loop every time is `O(NUMBER_OF_ENUMS)` where `NUMBER_OF_ENUMS` is a constant number. Thus it's the same as `O(1)` - *It's basic **runtime** analysis.* – jjnguy Mar 15 '11 at 18:39
3

@jjnguy: it's `O(n)` in the size of the enum. This is all pedantry, because the enum is unlikely to be large. – Matt Ball Mar 15 '11 at 18:41
@Matt, `O(n)` assumes that `n` can change. `n` is constant at runtime in this scenario so it becomes a constant runtime operation. – jjnguy Mar 15 '11 at 18:42
6

@jinguy, they might both be "constant" time operations, but the constant in each operation is different. One is the time to find a value in a hashtable, the other is the time required to loop through a variable (but constant at runtime) array of values. If you had one million values in this enum (not practical, but just an example) then you would prefer the map-lookup option. – matt b Mar 15 '11 at 18:42
5

@jjnguy: stating that an algorithm is `O(n)` does not imply that `n` can change _at runtime_. What if this performed an analogous lookup on an immutable (therefore fixed-at-runtime) list? That would absolutely be an `O(n)` algorithm, where `n` is the size of the list. – Matt Ball Mar 15 '11 at 18:43
@Matt, but that's what we are concerned with when we are talking about performance. See Paulo's answer. That is a `O(1)` lookup, and it is equivalent to looping through `values()`. – jjnguy Mar 15 '11 at 18:45
11

@jjnguy have a look at http://rob-bell.net/2009/06/a-beginners-guide-to-big-o-notation/ O(1) is "an algorithm that will always execute in the same time (or space) regardless of the size of the input data set." whereas O(N) is "an algorithm whose performance will grow linearly and in direct proportion to the size of the input data set" so while in this case the size of the data set doesn't change from run to run (why I think you consider it 'constant'), the performance of the algorithm is still based on the size of the input data set (in this case, the number of entries in the enum) – digitaljoel Mar 15 '11 at 18:55
2

I think in this case we should go and compare the absolute space and time characteristics for real-world enum sizes, not for `N → ∞`. – Paŭlo Ebermann Mar 15 '11 at 18:58
@Paŭlo: like I said in my answer, the `Map` will have higher overhead, and for small enums, a linear-time lookup will not have any noticeable performance impact. Like I said earlier in the comments, this is all pedantry. – Matt Ball Mar 15 '11 at 18:59
2

Actually, a linear scan could be a bit faster for typical enums with less then 10 members. However, there's a call to `values()` which makes it probably slower. – maaartinus Mar 15 '11 at 19:18
@jjinguy - Ah, getting the first item in a list vs. the last is not a constant speed operation (O(1)), even when the size of the set (n) doesn't change. Whether the size changes or not is irrelevant, it is the lookup operation that we are concerned with and in an unordered set that is O(n). Retrieving it from a hash map is a constant. The notation doesn't necessarily mean that one is faster than the other either, just that it is more predictable. – Robin Mar 15 '11 at 20:20
@Robin: _"getting the first item in a list vs. the last is not a constant speed operation"_ - that's not necessarily true. `ArrayList` accesses the `i`-th element of a list in constant time; `LinkedList` accesses the `i`-th element in `O(i)` time. The point is that _the list has to be traversed to find the element_ when using a `List` rather than a `Map`. – Matt Ball Mar 15 '11 at 20:27
1

@Matt Ball-obviously I was referring to the case in question, which is searching for an item in a list, not a fetch by index. Hence the reason I also specified "unordered" since the search time for an ordered list would also be better than O(n) – Robin Mar 16 '11 at 15:54
2

I've opted to return an `java.util.Optional` in my static lookup method. This wasn't available when this answer was written, but it makes it directly clear to the user that the lookup may fail. Maybe you could integrate that into your answer? I'll probably post an answer myself too, but it seems a good addition to yours as well. – Maarten Bodewes Dec 21 '18 at 22:40

score 7 · Answer 3 · answered Mar 15 '11 at 18:40

7

Here is an alternative which may be even a bit faster:

public enum Status {
    WAITING(0),
    READY(1),
    SKIPPED(-1),
    COMPLETED(5);

    private int code;

    private Status(int code) {
        this.code = code;
    }

    public int getCode() { return code; }

    public static Status get(int code) {
        switch(code) {
            case  0: return WAITING;
            case  1: return READY;
            case -1: return SKIPPED;
            case  5: return COMPLETED;
        }
        return null;
    }
}

Of course, this is not really maintainable if you want to be able to add more constants later.

answered Mar 15 '11 at 18:40

Paŭlo Ebermann

73,284
20
146
210

This is **exactly the same** as looping through `values()` at runtime as far as speed goes. – jjnguy Mar 15 '11 at 18:43
10

Not **exactly** the same, the switch version could use a lookup table to jump directly to the right code instead of doing a series of tests: http://www.artima.com/underthehood/flowP.html – Dan Berindei Mar 15 '11 at 18:51
12

@jjnguy: No, the compiler can optimize this switch to use a binary search, or a lookup table (depending on the numbers). And you don't need to create and populate the `values()` array before (which alone would make this variant `O(n)`). Of course, now the method is longer, so loading it takes longer. – Paŭlo Ebermann Mar 15 '11 at 18:54
@Paulo, ok. That makes sense I guess. – jjnguy Mar 15 '11 at 18:56
@Paulo this is neat if you **actually care** about the performance. I would rewrite it with `... case WAITING.code: ...` etc. and make sure there was a unit test checking all of `Status.values()` against the `get()` method and making sure none returned `null` and the expected `Status` was returned - this should take some of the worry out of maintainability. – Armand Mar 17 '11 at 10:58
1

@Alison: `case WAITING.code` is a nice idea, but I fear that this is not a compile-time constant. – Paŭlo Ebermann Mar 17 '11 at 11:49
@Paulo ah, fair point. Perhaps it would be fun to define `private static final int WAITING_CODE = 0` and use this in the `switch` and in the `enum` declaration?! – Armand Mar 17 '11 at 12:07
1

@jinguy I don't think they would have bothered creating the tableswitch instruction if they didn't want to use table lookup. Don't know if the lookupswitch instruction has any optimizations. – Dan Berindei Mar 17 '11 at 16:05
17

But this is an absolutely inacceptable solution in terms of clean code. With this solution you got two different places where the id is mapped to the enum value, so there can be bugs when the mappings differ! – Zordid Mar 27 '13 at 12:25

score 6 · Answer 4 · answered Mar 15 '11 at 18:36

6

Obviously the map will provide constant time lookup whereas the loop won't. In a typical enum with few values, I don't see a problem with the traversal lookup.

answered Mar 15 '11 at 18:36

digitaljoel

26,265
15
89
115

For a handful of enums, it doesn't really matter in my view. – endless Jun 10 '20 at 19:11

score 3 · Answer 5 · answered May 11 '15 at 14:17

Here is an Java 8 alternative (with unit test):

// DictionarySupport.java :

import org.apache.commons.collections4.Factory;
import org.apache.commons.collections4.map.LazyMap;

import java.util.HashMap;
import java.util.Map;

public interface DictionarySupport<T extends Enum<T>> {

    @SuppressWarnings("unchecked")
    Map<Class<?>,  Map<String, Object>> byCodeMap = LazyMap.lazyMap(new HashMap(), (Factory) HashMap::new);

    @SuppressWarnings("unchecked")
    Map<Class<?>,  Map<Object, String>> byEnumMap = LazyMap.lazyMap(new HashMap(), (Factory) HashMap::new);


    default void init(String code) {
        byCodeMap.get(this.getClass()).put(code, this);
        byEnumMap.get(this.getClass()).put(this, code) ;
    }

    static <T extends Enum<T>> T getByCode(Class<T> clazz,  String code) {
        clazz.getEnumConstants();
        return (T) byCodeMap.get(clazz).get(code);
    }

    default <T extends Enum<T>> String getCode() {
        return byEnumMap.get(this.getClass()).get(this);
    }
}

// Dictionary 1:
public enum Dictionary1 implements DictionarySupport<Dictionary1> {

    VALUE1("code1"),
    VALUE2("code2");

    private Dictionary1(String code) {
        init(code);
    }
}

// Dictionary 2:
public enum Dictionary2 implements DictionarySupport<Dictionary2> {

    VALUE1("code1"),
    VALUE2("code2");

    private Dictionary2(String code) {
        init(code);
    }
}

// DictionarySupportTest.java:     
import org.testng.annotations.Test;
import static org.fest.assertions.api.Assertions.assertThat;

public class DictionarySupportTest {

    @Test
    public void teetSlownikSupport() {

        assertThat(getByCode(Dictionary1.class, "code1")).isEqualTo(Dictionary1.VALUE1);
        assertThat(Dictionary1.VALUE1.getCode()).isEqualTo("code1");

        assertThat(getByCode(Dictionary1.class, "code2")).isEqualTo(Dictionary1.VALUE2);
        assertThat(Dictionary1.VALUE2.getCode()).isEqualTo("code2");


        assertThat(getByCode(Dictionary2.class, "code1")).isEqualTo(Dictionary2.VALUE1);
        assertThat(Dictionary2.VALUE1.getCode()).isEqualTo("code1");

        assertThat(getByCode(Dictionary2.class, "code2")).isEqualTo(Dictionary2.VALUE2);
        assertThat(Dictionary2.VALUE2.getCode()).isEqualTo("code2");

    }
}

Could you please explain your code instead of providing just a code dump? It looks like an OK implementation (actually, I've just programmed a similar solution myself), but without listing its properties people would have to evaluate it based on the code, and that's not likely to happen. — Maarten Bodewes, Dec 21 '18 at 22:32

score 1 · Answer 6 · answered Apr 21 '17 at 07:19

1

In Java 8 I would just add the following factory method to your enum and skip the lookup Map.

public static Optional<Status> of(int value) {
    return Arrays.stream(values()).filter(v -> value == v.getCode()).findFirst();
}

answered Apr 21 '17 at 07:19

ce72

128
2
4

Thoughts on this: Just a for loop under the covers. Not super readable. Might be interesting to write a reusable function to do this and have it take (value, Status::getCode) (like Maps.uniqueIndex does) – Barett Dec 13 '17 at 04:09
1

This is slower than any of the other answers mentioned above. 1. You're creating a new array every time from `values()` (this in itself is very slow, so if done a lot, you can speed this up quite a bit by caching the array once and reusing it), 2. You're using streams (which, in performance/benchmark tests, generally tests much slower than a simple for-loop). – Shadow Man Apr 15 '19 at 23:27

score 1 · Answer 7 · answered Dec 15 '20 at 23:58

@AllArgsConstructor
@Getter
public enum MyEnum {
    A(0),
    B(1),
    C(2);
    private static final Map<Integer, MyEnum> LOOKUP =
            Arrays.stream(MyEnum.values()).collect(Collectors.toMap(MyEnum::getStatus, Function.identity()));
    private final int status;

    @Nullable
    public static MyEnum fromStatus(int status) {
        return LOOKUP.get(status);
    }
}

score -2 · Answer 8 · answered Mar 15 '11 at 18:35

-2

Both ways are perfectly valid. And they have technically the same Big-Oh running time.

However, if you save all of the values to a Map first, you save the time it takes to iterate through the set each time you want to do a lookup. So, I think that the static map and initializer are a slightly better way to go.

answered Mar 15 '11 at 18:35

jjnguy

136,852
53
295
323

3

As the number of enum constants is and constant, everything is `O(1)` :-) – Paŭlo Ebermann Mar 15 '11 at 18:36
12

Nah, the linear lookup runs in linear time O(n) instead of O(1) for the `HashMap`. On the other hand, n is 4... – Tom Hawtin - tackline Mar 15 '11 at 18:37
@Paulo, great. That's what I thought. – jjnguy Mar 15 '11 at 18:37
@Tom, `n` isn't really `n`, it's some constant number that is fixed at runtime. – jjnguy Mar 15 '11 at 18:38
6

@jjnguy: The thing is, an increase in the number of constants is a linear increase in the runtime of the lookup, making the lookup O(N). That the number of constants doesn't change at runtime is immaterial. – ColinD Mar 15 '11 at 19:12
4

@jjnguy: that comment is meaningless. N is the number of items. Period. – user207421 Mar 16 '11 at 00:56
2

You should really correct the answer. Getting the *n*th element in the array is O(n). – user1803551 Sep 06 '18 at 09:33
1

It's a bit of a shame that this answer is still here, because no `for / next` loop is ever O(1), it's really that simple. There is no "big O running time" - the big O simply shows that you need to multiply against some value even the runtime of one iteration. Iterating through all the values in the enum is perfectly fine from a performance point of view, and that should be the answer. I prefer a map simply because it is more readable when implemented correctly; a map better communicates the intent: it is a table lookup. – Maarten Bodewes Dec 21 '18 at 22:37

Java enum reverse look-up best practice

8 Answers8

Linked

Related