Java enum best practice

Question

This might seem like a trivial question, but I'm a bit muddled in my thinking regarding enums..

So I have a class - let's say its called DVDPlayer - and I want to have an enum that represents whether it's ON, OFF, or STANDBY.

So I can put the enum in the class - it doesn't make sense outside of the class. My question is this - should the enum be public, so that other classes can query values, or should I make it private, and then have "isOn", "isOFf" and "isStandby" methods?

The latter sounds a bit daft, but I'm unsure as to whether it's a good idea to have the enum as public as well.

Answer 1

I would say it seems like a good idea to make it public. Main reason being that the application is easier to extend since you wouldn't have to think about adding new methods each time you add a state.

If you decide to make it public, you should consider having it as top-level enum. I don't really see why you say "it doesn't make sense outside of the class". I think a DVDPlayerState sounds like a perfectly fine public / top-level enum.

Answer 2

It depends on how you want to use the DVDPlayer class from the outside world:

if (dvdPlayer.getState() == State.ON)

or

if (dvdPlayer.isOn())

I think the first one is a better option. You don't have to pollute your code with delegating methods.

Answer 3

Making the enum public could make sense. You would then have something like this:

DvdPlayer.State getState();

If you only have three states, it may be preferable to use the isOn, isOff, and isStandby methods. For more states the public enum is better. Also an enum can be used in a switch statement, which is convenient.

Answer 4

As a rule of thumb you want to keep things as private as possible (though usually, that's not the use case with enums), but from the way your question is posed, I'm not sure you're using enums as intended.

You want to use an enum to represent fixed values; it's cleaner alternative to saving these values as static final Integers or Strings. So for an enum declared as

public enum DvdState { ON, OFF, STANDBY };

Your class would look a bit like this:

public class DvdPlayer {
    private DvdState state = DvdState.OFF;

    public void setState(DvdState state) {
        this.state = state;
    }
}

And a calling class would use the following code:

dvdPlayer.setState(DvdState.ON);