8

I've often run through a validation pattern where, to be valid, some variables must contain one of a prefixed number of values. 

PSEUDO CODE:
    IF x == CONSTANT_1 || X == CONSTANT_2 || ... || x == CONSTANT_N
    THEN X is valid

In order to avoid the chain of OR terms, I created a static final unmodifiable set, which contains all the constants:

public final static String CONSTANT_1 = *value* ;
public final static String CONSTANT_2 = *value* ;
...
public final static String CONSTANT_N = *value* ;

public final static Set SET_OF_CONSTANTS = Collections.unmodifiableSet(new HashSet(){
    private static final long serialVersionUID = 1L;
    {
        add(CONSTANT_1); 
        add(CONSTANT_2);
        ...
        add(CONSTANT_3);
    }
});

And I perform the check in the following way:

if(!SET_OF_CONSTANTS.contains(x)){ 
    //X NOT VALID 
}

I'd like to know if this is a good programming practice, if there are any alternatives, and if it's true that using a Hash Table query (O(1) in theory) instead of the OR terms-chain improves performance and maybe also code-readability.

Luigi Massa Gallerano
  • 2,347
  • 4
  • 23
  • 25
  • 5
    I think this is very good style. As far as performance goes, I wouldn't try to optimize anything until I've profiled the application on typical data. – NPE Dec 11 '12 at 12:33
  • +1 for NPE. Never presume where your performance bottleneck is and never optimize prematurely – Aniket Inge Dec 11 '12 at 12:34
  • 2
    Yes, it's true. There's also an alternative, EnumSet, which is implemented as bit set operations. Though it might not be usable as a in-place replacement, it could require API change. – Rekin Dec 11 '12 at 12:34
  • 1
    **Note:** Comparing strings with `==` is wrong most of the time. In most cases, you have to use `aString.equals(otherString)`. http://stackoverflow.com/questions/767372/java-string-equals-versus – Philipp Dec 11 '12 at 12:40
  • why not a hashmap since the constants are key value pairs ? – Bhavik Shah Dec 11 '12 at 12:41
  • Thanks Philipp, but I really know what's the difference between == and equals. The first part of my post, where I use ==, is to be intended as pseudo code and not Java code. Sorry if it wasn't clear. – Luigi Massa Gallerano Dec 11 '12 at 12:43

2 Answers2

17

Overall, I think this is very good style.

There's not a huge difference, but I'd personally define SET_OF_CONSTANTS like so:

      public final static String CONSTANT_1 = "*value*";
      public final static String CONSTANT_2 = "*value*";
              ...
      public final static String CONSTANT_N = "*value*";

      public final static Set<String> SET_OF_CONSTANTS = Collections.unmodifiableSet(
        new HashSet<String>(Arrays.asList(
              CONSTANT_1, 
              CONSTANT_2,
                      ...
              CONSTANT_N
              )));

It's not entirely clear to me whether you even need the separate CONSTANT_1 constants, or whether you can simply fold the values into SET_OF_CONSTANTS.

As far as performance goes, I would not start optimizing anything until I've profiled the code on real data.

Finally, note that when x is a string, the following is probably incorrect:

IF x == CONSTANT_1 || x == CONSTANT_2 || ... || x == CONSTANT_N

Here, the == should probably be replaced with calls to equals().

NPE
  • 486,780
  • 108
  • 951
  • 1,012
  • Thank you for the answer. Anyway I know what's the difference between == and equals. The first part of my post, where I use ==, is to be intended as pseudo code and not Java code. Sorry if it wasn't clear. I edited my post. – Luigi Massa Gallerano Dec 11 '12 at 12:48
  • (Or `==` is right and you should use `Collections.newSetFromMap(new IdentityHashSet<>(...))` but that leaves what to use for `...`. Unlikely, I admit.) – Tom Hawtin - tackline Dec 11 '12 at 12:50
2

Another (shorter) way would be to use Set.of : 

public final static Set SET_OF_CONSTANTS = Collections.unmodifiableSet(
    Set.of(CONSTANT_1, CONSTANT_2, CONSTANT_3));
ruben056
  • 112
  • 8