HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMajor

Java enum containing a hash map for look up

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
containingmapenumlookhashjavafor

Problem

I have the below enum. I need to get the description by code. It is working but can it be improved still?

public enum Maps {

    COLOR_RED("ABC", "abc description");

    private final String code;
    private final String description;
    private static Map mMap;

    private Maps(String code, String description) {
        this.code = code;
        this.description = description;
    }
    public String getCode() {
        return code;
    }

    public String getDescription() {
        return description;
    }

    public static String getDescriptionByCode(String code) {
        if (mMap == null) {
            initializeMapping();
        }
        if (mMap.containsKey(code)) {
            return mMap.get(code);
        }
        return null;
    }

    private static void initializeMapping() {
        mMap = new HashMap();
        for (Maps s : Maps.values()) {
            mMap.put(s.code, s.description);
        }
    }
}

Solution

Your code is very neat and clean, there is only one thing that I would change:

private static Map mMap;


to

private static final Map mMap = Collections.unmodifiableMap(initializeMapping());


The reasons:

-
By not declaring it final and not using a call to unmodifiableMap it is mutable, and it would be possible to modify the reference using reflection or to use .remove("ABC") on the map. Declaring it as final makes sure that the referenced map cannot change and unmodifiableMap makes sure that no changes can be done to the map itself.

-
Multi-threading issues. As it currently stands, if two threads would call the getDescriptionByCode method at the same time you would initialize the mapping twice, which is not needed.

This also of course requires a slight change in your initializeMapping():

private static Map initializeMapping() {
    Map mMap = new HashMap();
    for (Maps s : Maps.values()) {
        mMap.put(s.code, s.description);
    }
    return mMap;
}


Besides this, it all looks good. Well done!

Code Snippets

private static Map<String, String> mMap;
private static final Map<String, String> mMap = Collections.unmodifiableMap(initializeMapping());
private static Map<String, String> initializeMapping() {
    Map<String, String> mMap = new HashMap<String, String>();
    for (Maps s : Maps.values()) {
        mMap.put(s.code, s.description);
    }
    return mMap;
}

Context

StackExchange Code Review Q#46175, answer score: 40

Revisions (0)

No revisions yet.