6

I've got two enums: level with 3 values and criticality with 4 values. A combination of those two maps to one of 8 values from priority enum. The mapping is non-linear and may change in future.

What is the best* way to implement a static function that takes level and criticality and outputs a priority?

*best being easy to read and understand, easy and safe to change, and not a performance hog. Extra points for a solution that takes into account that input domain can change in the future.

Ways I considered so far:

nested switch..case. Many lines and lots of boilerplate code. Also error-prone if you forget to return a value in a case. Basically the code looks like this:

    switch (bc) {
        case C1:
            switch (el) {
                case E1:
                    return EmergencyPriority.P1;
                case E2:
                    return EmergencyPriority.P2;
                case E3:
                    return EmergencyPriority.P3;
            }
        case C2:
            switch (el) {
                case E1:
                    return EmergencyPriority.P2;
                case E2:
                    return EmergencyPriority.P3;
                case E3:
                    return EmergencyPriority.P4;
            }
        case C3:
            switch (el) {
                case E1:
                    return EmergencyPriority.P4;
                case E2:
                    return EmergencyPriority.P5;
                case E3:
                    return EmergencyPriority.P6;
            }
        case C4:
            switch (el) {
                case E1:
                    return EmergencyPriority.P6;
                case E2:
                    return EmergencyPriority.P7;
                case E3:
                    return EmergencyPriority.P8;
            }
    }

Mutikey Map requires an external library and I haven't found a way to nicely insert initial values without many function calls and boilerplate composite keys.


if..else if.. else basically same as switch case but with more boilerplate code. Less error-prone though.


Two dimensional array when using the enum values as integers for array indices you risk failing silently if the positional enum values change.


Your solution here

3 Answers 3

8

This structure is probably the "best" way to store your data ("best" = what I'm assuming your after, because I'd be perfectly fine with your switch based solution)

1. An EnumMap based solution

EnumMap<Level, EnumMap<Criticality, Priority>> map = new EnumMap<>(Level.class);
EnumMap<Criticality, Priority> c1 = new EnumMap<>(Criticality.class);
c1.put(Criticality.E1, Priority.P1);
..
map.put(Level.C1, c1);
...

Then, simply write this utility method to access the structure:

public static Priority priority(Level level, Criticality criticality) {
    return map.get(level).get(criticality);
}

The advantage of EnumMap is: It offers Map convenience while being rather efficient, as all the possible keys are known in advance, so values can be stored in an Object[].

2. An array based solution

You've already mentioned this, but I'll still repeat the idea, because I've done this in the past, and with proper formatting (that must never be broken by devs, of course), this approach is very readable and not very error prone.

Remember, formatting is key here:

Priority[][] map = {
  //               Criticality.E1   Criticality.E2   Criticality.E3
  // ----------------------------------------------------------------
  /* Level.C1 */ { Priority.P1    , Priority.P2    , Priority.P3    },
  /* Level.C2 */ { Priority.P2    , Priority.P3    , Priority.P4    },
  /* Level.C3 */ { Priority.P3    , Priority.P4    , Priority.P5    },
  /* Level.C4 */ { Priority.P4    , Priority.P5    , Priority.P6    }
};

And now, the method looks like this:

public static Priority priority(Level level, Criticality criticality) {
    return map[level.ordinal()][criticality.ordinal()];
}

In order to prevent failing silently in case someone adds a new enum value in the middle, just add a unit test that asserts the expected ordinal for each enum literal. The same test can also assert the Level.values().length and Criticality.values().length values, and you're safe for the future.

Sign up to request clarification or add additional context in comments.

3 Comments

Interesting. First time I hear about EnumMap. However, I think this solution is hard to read at first glance since you have to read three full lines of code in order to see one connection. Also this solution will fail with a runtime error if one of the two input enums are expanded and an unmapped value is given as param.
@HubertGrzeskowiak: I told you I'd be perfectly fine with your switch based solution :) (there's IDE support for missing case statements). You didn't specify what should happen in the case of a missing mapping.
@HubertGrzeskowiak: Ok, I added another argument in favour of the array based solution.
1

1. Define an enum with constructors

Inspired by the comment on EnumMap using Object[], I came up with this solution:

public enum EmergencyPriority {
    P1(BusinessCriticality.C1, EmergencyLevel.E1), 
    P2(BusinessCriticality.C1, EmergencyLevel.E2, 
       BusinessCriticality.C2, EmergencyLevel.E1), 
    P3(BusinessCriticality.C1, EmergencyLevel.E3, 
       BusinessCriticality.C2, EmergencyLevel.E2), 
    P4(BusinessCriticality.C2, EmergencyLevel.E3, 
       BusinessCriticality.C3, EmergencyLevel.E1), 
    P5(BusinessCriticality.C3, EmergencyLevel.E2), 
    P6(BusinessCriticality.C3, EmergencyLevel.E3, 
       BusinessCriticality.C4, EmergencyLevel.E1), 
    P7(BusinessCriticality.C4, EmergencyLevel.E2), 
    P8(BusinessCriticality.C4, EmergencyLevel.E3);

    private static EmergencyPriority[][] PRIORITIES;

    private EmergencyPriority(BusinessCriticality c, EmergencyLevel l) {
        addPriority(l, c, this);
    }

    private EmergencyPriority(BusinessCriticality c, EmergencyLevel l, 
            BusinessCriticality c2, EmergencyLevel l2) {
        addPriority(l, c, this);
        addPriority(l2, c2, this);
    }

    private static void addPriority(EmergencyLevel l, BusinessCriticality c, EmergencyPriority p) {
        if (PRIORITIES == null) {
            PRIORITIES = new EmergencyPriority[EmergencyLevel.values().length][BusinessCriticality.values().length];
        }
        PRIORITIES[l.ordinal()][c.ordinal()] = p;
    }

    public static EmergencyPriority of(BusinessCriticality c, EmergencyLevel l) {
        return PRIORITIES[l.ordinal()][c.ordinal()];
    }
}

2. Define an enum and statically initialise the correspondance array

Another solution would be to have a simple enum with no constructors and statically initialising the priorities array, which enables to reorder them as you see fit for readability:

import static com.package.BusinessCriticality.*;
import static com.package.EmergencyLevel.*;

public enum EmergencyPriority {
    P1, P2, P3, P4, P5, P6, P7, P8;

    private static EmergencyPriority[][] PRIORITIES = new EmergencyPriority[BusinessCriticality.values().length][EmergencyLevel.values().length];

    private void define(BusinessCriticality c, EmergencyLevel e) {
        PRIORITIES[c.ordinal()][e.ordinal()] = this;
    }

    static {
        P1.define(C1, E1);

        P2.define(C1, E2);
        P2.define(C2, E1);

        P3.define(C1, E3);
        P3.define(C2, E2);

        P4.define(C2, E3);
        P4.define(C3, E1);

        P5.define(C3, E2);

        P6.define(C3, E3);
        P6.define(C4, E1);

        P7.define(C4, E2);

        P8.define(C4, E3);
    }

    public static EmergencyPriority of(BusinessCriticality c, EmergencyLevel e) {
        return PRIORITIES[c.ordinal()][e.ordinal()];
    }
}

And you could have the following JUnit test to ensure that EmergencyPriority has all the combinations, in case you extend BusinessCriticality or EmergencyLevel:

@Test
public void testEnumCompletude() {
    for (BusinessCriticality c : BusinessCriticality.values()) {
        for (EmergencyLevel e : EmergencyLevel.values()) {
            assertNotNull(String.format("%s/%s combination was forgotten", c, e), 
                EmergencyPriority.of(c, e));
        }
    }
}

9 Comments

This is very interesting. The boilerplate is above the switch...case solution but this looks more readable and is more OOP. I like how you're using the lightweight two dimensional array internally while not relying on the Enums' ordinal values. Also +1 for showing me the first viable situation of a useful private static method. Only thing that might confuse the reader is the overloaded constructor: you pass two pairs of arguments as four separate args. With some formatting (newline after a pair?) this is my favorite so far.
@HubertGrzeskowiak Thank you. The fact that I had to declare 2 constructors annoys me too but I don't see a better way of doing it with Java not supporting tuples natively. I added a newline after the first pair and I think it looks okay this way.
@Valentin java doesn't have a tuple class because is just too easy to make one, you should make a private one inside your EmergencyPriority enum structure and, instead of have many constructors, create a constructor with a varargs initializacion... something like... private EmergencyPriority(Tuple... tuples){...}, with minor changes, your code should still working...
@AlexandroSifuentesDíaz Sure it's easy to define a tuple class (in almost all languages) but having to call their constructor adds a lot of characters to the code (which I tried to make simple). If tuples were a language feature, I could have a single constructor and the constructor calls wouldn't be too verbose with something like private EmergencyPriority((BusinessCriticality, EmergencyLevel)... values) { values.forEach((c, l) -> addPriority(c, l, this)); }
@HubertGrzeskowiak I came up with another solution, which looks cleaner to me, derived from my first solution.
|
1

Maybe a Map with a tuple of Criticality and Level as a key?

You could create a key class with a custom equal() and hashCode() method that encapsulate this two values like the following:

public class PriorityTuple{    
  final Criticality c;
  final Level l;

  public PriorityTuple(Criticality c, Level l) {
    this.c = c;
    this.l = l;
  }

  @Override
  public boolean equals(Object o) {
    if (!(o instanceof PriorityTuple)) {
      return false;
    }
    PriorityTuple prioritykey = (PriorityTuple) o;
    return this.c.equals(prioritykey.c) && this.l.equals(prioritykey.l);
  }

  @Override
  public int hashCode() {
    int hash = 5;
    hash = 23 * hash + Objects.hashCode(this.c);
    hash = 23 * hash + Objects.hashCode(this.l);
    return hash;
  }
}

And then create your Map with your entities:

Map<PriorityTuple, Priority> priorityMap = new HashMap<>();

And two methods to simplify add and get:

// add new entry
public static void addPriority(Criticality c, Level l, Priority p) {
  if (null == c || null == l || null == p) return; // if you want some kind of control
  priorityMap.put(new PriorityTuple(c, l), p);
}
// get priority
public static Priority priority(Criticality c, Level l) {
  return priorityMap.get(new PriorityTuple(c, l));
}

Resulting in something like:

addPriority(Criticality.C1, Level.L1, Priority.P1);
addPriority(Criticality.C1, Level.L2, Priority.P2);
addPriority(Criticality.C1, Level.L3, Priority.P3);
addPriority(Criticality.C2, Level.L1, Priority.P2);
addPriority(Criticality.C2, Level.L2, Priority.P3);
addPriority(Criticality.C2, Level.L3, Priority.P4);
// and so on...

// retrieving values...
System.out.println(priority(Criticality.C1, Level.L1)); // print P1
System.out.println(priority(Criticality.C4, Level.L3)); // null if not exist

With this, you can continue add more entries for your enum types in the future without breaking the code (?)

1 Comment

I really like how the actual logic (last code block) is separated from all the boilerplate. And you can easily extend this to catch unexpected Enum values in priority(c, l).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.