0

I have a 2 enum values representing a mapping to object which I'm (currently) modeling with a HashMap with the 2 enums values are used as key and the object is the value.

This is inefficient because what I'm doing is creating a new CompositeKey(Enum1 enum1, Enum2 enum2) for each combination of the Enum1.values() x Enum2.values().

I would like to skip the new CompositeKey() issue.

The solution I'm currently think of is Calculation a number representations of from the 2 enums, something like int numericKey = enum1.ordinal() * 0xFFFF + enum2.ordinal(); but then when I'll do map.get(numricKey) I will still be auto boxing to Integer - hence creation new instances.

A perfect solution (IMO) would be Map implementation (doesn't have to be generic...) but I don't think such existing for java.

Another option is probably mapping = new Object[Enum1.values().length][Enum2.values().length] which I will then do lookup with Object = mapping[enum1.ordinal()][enum2.ordinal()] but that seems a bit too "C'ish".

Anyways, runtime performance is the most important aspect here.

Comments are welcome.

Thanks, Maxim.

Maxim Veksler
  • 25,736
  • 34
  • 119
  • 148

4 Answers4

5

Using the ordinal of an enum is a very bad idea because the ordinal is an internal representation of an enum and should never be used in external code. The Enum specification has this to say about ordinal:

Most programmers will have no use for this method. It is designed for use by sophisticated enum-based data structures, such as EnumSet and EnumMap.

I recommend using an EnumMap which is specifically designed such purposes as the one you have. Create an EnumMap<Enum1,EnumMap<Enum2,V>> and populated it using the values of your enums:

for (Enum1 e1: Enum1.values()) {
    map.put(e1, new EnumMap<Enum2,V>());
    for (Enum2 e2 : Enum2.values()) {
        map.get(e1).put(e2, getSomeValue(e1, e2));
    }
}
Abhinav Sarkar
  • 22,313
  • 10
  • 78
  • 95
  • That is a very good idea which I simply haven't thought of. Thanks. – Maxim Veksler Sep 16 '10 at 22:41
  • I'd create the `new EnumMap()` as a variable which would avoid the `map.get(e1)` over and over getting the same value as the `new` in the outer loop. – Gray Sep 01 '11 at 18:31
  • ordinal() of enum is **absolutely fine** to use, provided you don't try and store it outside the JVM & expect it to be guaranteed readable in your next software version. **ordinal() is there for exactly this sort of use -- to implement performant lookups & structures**. -1 for the incorrect answer. – Thomas W May 06 '13 at 04:00
1

I think the CompositeKey approach is the best from a maintenance perspective, because it lets you use the existing collections unchanged. The overhead of creating instances of this object really isn't that high -- most of the instances will be short-lived and never get out of the Eden space (which is both allocated and collected very quickly).

You could also implement Map<Enum1,Map<Enum2,?>>, with EnumMap as your implementation type. However, you'll have to either pre-populate the "outer" maps, or write code to lazily populate it.

But if runtime performance is your top priority, then your idea of using an array is best. I wouldn't implement it as a two-dimensional array, however. Instead, use a one-dimensional array and index with the following:

int index = enum1.ordinal() * _enum1Size + enum2.ordinal;

Wrap it in a class and you're done.

Anon
  • 1,901
  • 12
  • 6
1

I disagree with Sarkar's point on the use of Enum.ordinal() method. It is effective and valid to use. Of course, this is based entirely on the scope of use. If you can apply the use of EnumMap or EnumSet in your code, then any solution you devise will have the same runtime scope. You should avoid use of the ordinal() method outside the current running VM, e.g. serialization, externalize or a persistent value.

The fastest solution still remains the calculated offset within an array, but could use potentially large amounts of memory. But if you have large enumerations (which would really call into doubt the appropriate use of an Enum) with very few combinations, resulting in a large but sparse array, then your original "compound" key is a very close second.

Finally, purely for performance, if you are forming the compound key given in your example, int numericKey = enum1.ordinal() * 0xFFFF + enum2.ordinal(); use the faster shift operator and OR operator rather than the multiply and add operators: int numericKey = (enum1.ordinal() << 16) | enum2.ordinal();

Banner
  • 11
  • 2
  • 1
    Just FYI but your faster shift operator results in less than a 1% speed improvement over 100 _million_ iterations. Also, I think you meant | (or) instead of & (and). Respectfully. – Gray Sep 01 '11 at 18:41
  • Thanks for the AND-OR correction, I will update the response. I actually get slightly over 1% ;) I'll take it. – Banner Sep 01 '11 at 19:33
  • I don't take it. All such code does is obfuscate with the promise of optimization which it rarely delivers. We need to leave our C coding style behind. Writing clean code will be more efficient from an overall SE statement in the long run. :-) – Gray Sep 01 '11 at 19:36
  • 1
    My premise was purely for performance, but since you want to consider this an issue of clean code, then it only bolsters my case. The approach should hardly be considered obfuscation as it is actually cleaner and self documenting. If you are bit-banging an integer field to form a compound value, then use bitwise operators to do the job. Don't pound nails with a wrench; you can do it, but it makes you look silly. Just FYI but C coding style has little to do with bitwise operators. Respectfully. – Banner Sep 01 '11 at 20:09
  • Bit banging originates from assembly then C, regardless of the language. I figured you weren't a .asm hacker. '# * # + #' is understood by a lot more people than '(# << #) | #'. One is basic math and the other is bit banging. Ex: http://stackoverflow.com/questions/3845018/python-boolean-operators-vs-bitwise-operators Then you have the hidden bugs with overflows and sign issues. All that plus the fact that the performance "improvement" is minuscule at best translates into a deprecated practice. YMMV. – Gray Sep 01 '11 at 20:49
  • Uncle! Too many things to fix in that last statement. Good luck. – Banner Sep 01 '11 at 22:47
  • Performance was the stated goal, `(v1.ordinal() << 16) + v2.ordinal` is the most efficient composite key. Put the 'composite key generation' in it's own function with a _comment_, for God's sake, and it's explained for evermore. This is the correct answer. – Thomas W May 06 '13 at 04:04
0

I have a HashMap named labels with a key of type Key with two enum as members. I noticed that two instances of Key created with same values had different hash codes. Overriding hashCode() I am able to get the corrisponding value, otherwise I always got null.

Here is the code:

public static String getMeasureLabel(MeasureSerie measure, TwampMeasure traffic) {
        Key key = new Key(traffic, measure);
        //Key key1 = new Key(traffic, measure); //key.hashCode() != key1.hashCode()
        return labels.get(key); // always returned null
    }

And here is Key

public static class Key {

        public TwampMeasure trafficType;
        public MeasureSerie measureType;


        @Override
        public int hashCode() {
            final int prime = 31;
            int result = 1;
            result = prime * result
                    + ((measureType == null) ? 0 : measureType.getValue().hashCode());
            result = prime * result
                    + ((trafficType == null) ? 0 : trafficType.getValue().hashCode());
            return result;
        }

    }
saralago
  • 1
  • 1