49

I have an enum:

public enum Persons {

    CHILD,
    PARENT,
    GRANDPARENT;

}

Is there any problem with using ordinal() method to check "hierarchy" between enum members? I mean - is there any disadvantages when using it excluding verbosity, when somebody can change accidentally order in future.

Or is it better to do something like that:

public enum Persons {

    CHILD(0),
    PARENT(1),
    GRANDPARENT(2);

    private Integer hierarchy;

    private Persons(final Integer hierarchy) {
        this.hierarchy = hierarchy;
    }

    public Integer getHierarchy() {
        return hierarchy;
    }

}
ByeBye
  • 6,650
  • 5
  • 30
  • 63

11 Answers11

77

TLDR: No, you should not!

If you refer to the javadoc for ordinal method in Enum.java:

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

Firstly - read the manual (javadoc in this case).

Secondly - don't write brittle code. The enum values may change in future and your second code example is much more clear and maintainable.

You definitely don't want to create problems for the future if a new enum value is (say) inserted between PARENT and GRANDPARENT.

Laurent
  • 14,122
  • 13
  • 57
  • 89
vikingsteve
  • 38,481
  • 23
  • 112
  • 156
  • 25
    How is having to manually renumber all subsequent items when you insert a new item in the list more maintainable? – David Moles Jun 20 '17 at 16:47
  • 2
    @DavidMoles - good question. Since you are explicitly declaring `hierarchy`, its clear you are associating an `Integer` with each enum. Most importantly, if the enums are somehow re-ordered (say, someone adds `GUARDIAN` on the line between `PARENT` and `GRANDPARENT`) then the value for the last enum value won't change. – vikingsteve Jun 20 '17 at 17:15
  • 8
    @DavidMoles It may not be the *most* maintainable, but it's clearly better than having your behavior depend on the order in which you happen to have declared your enum values. – Chris Hayes Jun 20 '17 at 19:24
  • 5
    @DavidMoles because the only thing less maintainable is code that will break if anyone changes it in the wrong way, and which adds nothing to explain that fact. Though personally, I would use the first method and adding a comment explaining that order matters... – nhouser9 Jun 20 '17 at 19:27
  • 9
    @nhouser9 ... and a set of tests that will fail if someone does ignore the comment and changes the order. If you're in the unfortunate position of having a team where unit tests are not run or failures are frequently ignored and implementation that explicitly numbers them is much safer. – Dan Is Fiddling By Firelight Jun 20 '17 at 19:51
  • 7
    @ChrisHayes Lots of things depend on the order in which you happen to have declared your enum values, including `compareTo()`, which is `final`. Order is meaningful for Java enums. If you want it not to be meaningful, you should write your own enum-like class with your own ordering rules. – David Moles Jun 20 '17 at 20:39
  • @vikingsteve If the intent is to allow multiple instances to share the same level, then clearly `ordinal()` wouldn't work in the first place. – David Moles Jun 20 '17 at 20:53
  • 2
    There are many cases where to avoid ordinal, and generally is a maintenance issue due to inherent ambiguity of just "ordinal" within any context. Joshua Bloch in Effective Java discusses this, as well as many academic articles/ papers – Psyrus Aug 29 '18 at 09:36
  • Side note : misuse of ordinal within java is widespread. I agree with above answer – Peeyush Mar 09 '19 at 05:05
  • @vikingsteve Ordinal is a very powerful function. For example if you had a MONTH Enum instance passed to a function which had to print out what # month of the year that month was, the only viable option available would be to use the `ordinal()` function. Calling `values()` is not typically a good practice as it must create an additional copy of the array on every call, which is O(n) space and complexity. Beginners could get the wrong impression from your answer. – Hatefiend Feb 04 '21 at 08:59
  • By default, HQL sets enumeration values using the ordinal() method in table columns. – Bruno L. Sep 19 '21 at 18:05
17

As suggested by Joshua Bloch in Effective Java, it's not a good idea to derive a value associated with an enum from its ordinal, because changes to the ordering of the enum values might break the logic you encoded.

The second approach you mention follows exactly what the author proposes, which is storing the value in a separate field.

I would say that the alternative you suggested is definitely better because it is more extendable and maintainable, as you are decoupling the ordering of the enum values and the notion of hierarchy.

msf1013
  • 369
  • 2
  • 10
15

The first way is not straight understandable as you have to read the code where the enums are used to understand that the order of the enum matters.
It is very error prone.

public enum Persons {

    CHILD,
    PARENT,
    GRANDPARENT;

}

The second way is better as it is self explanatory :

CHILD(0),
PARENT(1),
GRANDPARENT(2);

private SourceType(final Integer hierarchy) {
    this.hierarchy = hierarchy;
}

Of course, orders of the enum values should be consistent with the hierarchical order provided by the enum constructor arguments.

It introduces a kind of redundancy as both the enum values and the arguments of the enum constructor conveys the hierarchy of them.
But why would it be a problem ?
Enums are designed to represent constant and not frequently changing values.
The OP enum usage illustrates well a good enum usage :

CHILD, PARENT, GRANDPARENT

Enums are not designed to represent values that moves frequently.
In this case, using enums is probably not the best choice as it may breaks frequently the client code that uses it and besides it forces to recompile, repackage and redeploy the application at each time an enum value is modified.

davidxxx
  • 125,838
  • 23
  • 214
  • 215
12

First, you probably don't even need a numeric order value -- that's what Comparable is for, and Enum<E> implements Comparable<E>.

If you do need a numeric order value for some reason, yes, you should use ordinal(). That's what it's for.

Standard practice for Java Enums is to sort by declaration order, which is why Enum<E> implements Comparable<E> and why Enum.compareTo() is final.

If you add your own non-standard comparison code that doesn't use Comparable and doesn't depend on the declaration order, you're just going to confuse anyone else who tries to use your code, including your own future self. No one is going to expect that code to exist; they're going to expect Enum to be Enum.

If the custom order doesn't match the declaration order, anyone looking at the declaration is going to be confused. If it does (happen to, at this moment) match the declaration order, anyone looking at it is going to come to expect that, and they're going to get a nasty shock when at some future date it doesn't. (If you write code (or tests) to ensure that the custom order matches the declaration order, you're just reinforcing how unnecessary it is.)

If you add your own order value, you're creating maintenance headaches for yourself:

  1. you need to make sure your hierarchy values are unique
  2. if you add a value in the middle, you need to renumber all subsequent values

If you're worried someone could change the order accidentally in the future, write a unit test that checks the order.

In sum, in the immortal words of Item 47: know and use the libraries.


P.S. Also, don't use Integer when you mean int.

David Moles
  • 48,006
  • 27
  • 136
  • 235
  • enum fields may be used to other things besides comparison and order. – Leonardo Pina Jun 20 '17 at 16:52
  • @LeonardoPina Yes, but that's not what the OP asked. – David Moles Jun 20 '17 at 16:53
  • To be fair, it was unclear what OP wanted to do with 'hierarchy'. – Leonardo Pina Jun 20 '17 at 16:55
  • It's not a particularly well-written question, but the only use it explicitly talks about is order, and the example given exactly hard-codes the same values `ordinal()` would give. – David Moles Jun 20 '17 at 16:57
  • 2
    @Davide Moles. Thank you. A voice of reason in a sea of obnoxious and counterproductive lore . One always struggles with the sort of advice that's been given on this question. I spent two days mulling it over, and eventually came to the same conclusion you did. And was on my way back to write an argument against the use of enum fields in this case, when I found your post. If only I'd noticed it two days ago. – Robin Davies Jun 13 '18 at 00:00
8

If you only want to create relationships between enum values, you can actually use the trick of using other enum values:

public enum Person {
  GRANDPARENT(null),
  PARENT(GRANDPARENT),
  CHILD(PARENT);

  private final Person parent;

  private Person(Person parent) {
    this.parent = parent;
  }

  public final Parent getParent() {
    return parent;
  }
}

Note that you can only use enum values that were declared lexically before the one you're trying to declare, so this only works if your relationships form an acyclic directed graph (and the order you declare them is a valid topological sort).

Daniel Pryden
  • 59,486
  • 16
  • 97
  • 135
6

Using ordinal() is unrecommended as changes in the enum's declaration may impact the ordinal values.

UPDATE:

It is worth noting that the enum fields are constants and can have duplicated values, i.e.

enum Family {
    OFFSPRING(0),
    PARENT(1),
    GRANDPARENT(2),
    SIBLING(3),
    COUSING(4),
    UNCLE(4),
    AUNT(4);

    private final int hierarchy;

    private Family(int hierarchy) {
        this.hierarchy = hierarchy;
    }

    public int getHierarchy() {
        return hierarchy;
    }
}

Depending on what you're planning to do with hierarchy this could either be damaging or beneficial.

Furthermore, you could use the enum constants to build your very own EnumFlags instead of using EnumSet, for example

Leonardo Pina
  • 458
  • 1
  • 7
  • 17
3

I would use your second option (using a explicit integer) so the numeric values are assigned by you and not by Java.

gpeche
  • 21,974
  • 5
  • 38
  • 51
2

Let's consider following example:

We need to order several filters in our Spring Application. This is doable by registering filters via FilterRegistrationBeans:

 @Bean
  public FilterRegistrationBean compressingFilterRegistration() {
    FilterRegistrationBean registration = new FilterRegistrationBean();
    registration.setFilter(compressingFilter());
    registration.setName("CompressingFilter");
    ...
    registration.setOrder(1);
    return registration;
  }

Let's assume we have several filters and we need to specify their order (e.g. we want to set as first the filter which add do MDC context the JSID for all loggers)

And here I see the perfect usecase for ordinal(). Let's create the enum:

   enum FilterRegistrationOrder {
    MDC_FILTER,
    COMPRESSING_FILTER,
    CACHE_CONTROL_FILTER,
    SPRING_SECURITY_FILTER,
    ...
    }

Now in registration bean we can use: registration.setOrder(MDC_FILTER.ordinal());

And it works perfectly in our case. If we haven't had an enum to do that we would have had to re-numerate all filters orders by adding 1 to them (or to constants which stores them). When we have enum you only need to add one line in enum in proper place and use ordinal. We don't have to change the code in many places and we have the clear structure of order for all our filters in one place.

In the case like this I think the ordinal() method is the best option to achieve the order of filters in clean and maintainable way

2

You must use your judgement to evaluate which kind of errors would be more severe in your particular case. There is no one-size-fits-all answer to this question. Each solution leverages one advantage of the compiler but sacrifices the other.

If your worst nightmare is enums sneakily changing value: use ENUM(int)

If your worst nightmare is enum values becoming duplicated or losing contiguousness: use ordinal.

Agent_L
  • 4,960
  • 28
  • 30
1

According to java doc

Returns the ordinal of this enumeration constant (its position in its enum declaration, where the initial constant is assigned an ordinal of zero). 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.

You can control the ordinal by changing the order of the enum, but you cannot set it explicitly.One workaround is to provide an extra method in your enum for the number you want.

enum Mobile {
   Samsung(400), Nokia(250),Motorola(325);

   private final int val;
  private Mobile (int v) { val = v; }
  public int getVal() { return val; }
}

In this situation Samsung.ordinal() = 0, but Samsung.getVal() = 400.

gati sahu
  • 2,576
  • 2
  • 10
  • 16
1

This is not a direct answer to your question. Rather better approach for your usecase. This way makes sure that next developer will explicitly know that values assigned to properties should not be changed.

Create a class with static properites which will simulate your enum:

public class Persons {
    final public static int CHILD = 0;
    final public static int PARENT = 1;
    final public static int GRANDPARENT = 2;
}

Then use just like enum:

Persons.CHILD

It will work for most simple use cases. Otherwise you might be missing on options like valueOf(), EnumSet, EnumMap or values().

mzmrk
  • 109
  • 1
  • 4
  • The huge downside of that being that arguments for method declarations are of type int, and there is no longer compiler protection against passing an illegal value. This seems like a disproportionate price to pay for avoiding the incredibly subtle downside of using ordinal. – Robin Davies Jun 11 '18 at 15:37
  • IMO, that's for enums exist – olyv Feb 12 '19 at 08:19