42

Let's say I have an enum with 100 values. For simplicity's sake, take the following example:

public enum code
{
    CODE_1("string1"),
    CODE_2("string2"),
    CODE_3("string3"),
    CODE_4("string4"),
    ...
}

I want to create a public method to convert strings with a known format (like "string1", "string2"...) to the appropiate enum value CODE_1, CODE_2... Typically this is done by iterating over all values, and if a match is found, return that enum value. (Details can be found in this question.)

However, I'm concerned with reguraly looping over all values. Could this potentially be a huge bottleneck? What if instead of 100 element, there were 1000?

As an exercise for myself, I tried to optimize this lookup with a static map, which can assure O(1) lookup time given any string. I like this extra gimmick, but I only want to include it in my code if it is actually necessary. What are your thoughts and findings on using the iterating method vs the map method?

public enum Code
{
    ...
    //enum values
    ...


    //The string-to-Code map
    private static final Map<String,Code> CODE_MAP = populateMap();

    private static Map<String,Code> populateMap()
    {
        Map<String,Code> map = new HashMap<String,Code>();

        for(Code c : Code.values())
        {
            map.put(c.getCode(), c);
        }

        return map;
    }


    private String code;

    private Code(String code)
    {
        this.code = code;
    }

    public String getCode()
    {
        return this.code;
    }

    public Code convertFromString(String code)
    {
        //assume that the given string is actually a key value in the map

        return (Code) CODE_MAP.get(code);
    }
}
Community
  • 1
  • 1
user1884155
  • 3,616
  • 4
  • 55
  • 108
  • Have you profiled and found the naive, iterative way was a bottleneck? If you haven't, you don't need to worry about it - focus your attention on the code that's actually a bottleneck. – corsiKa Dec 30 '14 at 20:23
  • As stated in the question, this is a purely hypothetical exercise. It is not meant as a pre-optimilization. Thanks for your concern though – user1884155 Dec 30 '14 at 20:33
  • You have not oresented any evidence that the Java `vakueOf` method is too slow. So this is a duplicate. – Raedwald Jan 01 '15 at 09:52
  • Note: the reason I closed this question is that in fact, `valueOf` uses a `Map` and is therefore the correct method. – RealSkeptic Nov 16 '15 at 19:12
  • 2
    @RealSkeptic I'm really disappointed. ValueOf has nothing to do with this question, because valueof only works on strings that match the name of the enum. The example provided at the start of this question clearly shows a situation where the string name is very different from the enum code name. This is the same faulty reasoning Raedwald applied when he marked this question as duplicate. You CANNOT use valueOf in this situation! – user1884155 Dec 12 '16 at 13:30

5 Answers5

49

You want a Map<String, Code>, but how to populate it neatly? Enums don't allow you to initialize a static fields before the enum instances are initialized, but there's a neat little trick, called the Initialization-on-demand holder idiom, that makes using a statically initialized map needed for this functionality easy to implement:

public enum Code {
    CODE_1("string1"),
    CODE_2("string2"),
    CODE_3("string3"),
    // etc
    ;

    private static class Holder {
        static Map<String, Code> CODE_MAP = new HashMap<>();
    }

    private final String code;

    private Code(String code) {
        this.code = code;
        Holder.CODE_MAP.put(code, this);
    }

    public String getCode() {
        return this.code;
    }

    public Code convertFromString(String code) {
        return Holder.CODE_MAP.get(code);
    }
}

This works because the class loader initializes inner static classes before initializing the enum class, so the map is assigned ready to load during enum instance initialization.

No loops. No special code to load the map (done in constructor). Minimal code.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
  • 2
    So you're saying that the way I populate it in my code above is not "neat"? Could you explain why it is not "neat"? You also say "Enums don't allow you to initialize a static fields before the enum instances are initialized", but I don't think I'm doing that. I'm populating the map after my enum instances? – user1884155 Dec 30 '14 at 11:10
  • 3
    @user1884155 your code is not neat because there's so much if it - extra methods, more lines. This cude uses just 1 line for map initialization (or 2 lines if you count the `class Holder` line) and just 1 line to load it, with no extra methods defined - that would seem reasonable to call "neater". As for enums not allowing static initialuzation, I mean you can't do this: `private static Map CODE_MAP = new HashMap<>();` - if you do and try to use it, it will be null during enum instance initialization, so you can't load it neatly as I did from the enum's constructor. – Bohemian Dec 30 '14 at 12:42
  • @Bohemian Is the inner static class strictly necessary? A similar solution in [How to do reverse lookup in enums](http://howtodoinjava.com/2012/12/07/guide-for-understanding-enum-in-java/#reverse_lookup) makes the map `private static final` directly in the enum. – Greg Bacon Dec 30 '14 at 21:35
  • 3
    @greg your linked articke's code uses a static block to populate the map, which is required for a plain static map because the static map is not initialized until *after* all enum instances are initialized - too late to use the constructor to self-load each instance, which is much neater. Also static blocks are considered poor style for a few reasons, one being that if you move the static block above the map declaration it will compile but throw a NPE. – Bohemian Dec 31 '14 at 00:35
  • Great answer! Maybe I'm missing something but don't you need to access object Holder in method convertFromString for the example to compile? – RubioRic May 14 '21 at 07:25
  • 1
    @RubioRic yes! Good pickup. Corrected the missing `Holder` qualifier. Cheers – Bohemian May 14 '21 at 07:49
4

Map is good option : cleaner code and O(1) . If you use for-loop then the best you get is O(n)

sol4me
  • 15,233
  • 5
  • 34
  • 34
4

Your provided solution is proper implementation.

As you will have to expose only one method and it is more readable.

And it is always good to use Map instead of iterating it manually.

And also as you mentioned the complexity is O(1).

+1 to your question, as it gives a cleaner approach to use enum in some usecases.

Naman Gala
  • 4,670
  • 1
  • 21
  • 55
4

If your string code value is a known and consistent format you could avoid the use of a Map and the memory it consumes and construct the CODE enum lookup value on the fly:

  public static Code convertFromString(String code) {
    return valueOf("CODE_" + code.substring("string".length()));
  }
darrenmc
  • 1,721
  • 1
  • 19
  • 29
3

Well, alternatives to the map-solution would be a giant switch-statement (could be automatically generated) or binary-searching an array containing the strings. I don't think either will beat HashMap's performance by a large margin, though if it really matters, you are probably best off by benchmarking.

One thing that has not been mentioned is how Enum.valueOf() let's you turn a String into an enum value, if it has the exact name of one of the enum members. If this is at all a possiblity in your case (looking just at your example, I don't see how Code.CODE_1 could not be easily renamed Code.string1 etc.), I would suggest using it, as it requires no additional coding at all and will hence be the most understandable.

Benjaminssp
  • 243
  • 1
  • 2
  • 9