patternjavaMajor
Java enum containing a hash map for look up
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:
to
The reasons:
-
By not declaring it final and not using a call to
-
Multi-threading issues. As it currently stands, if two threads would call the
This also of course requires a slight change in your
Besides this, it all looks good. Well done!
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.