Buhake Sindi
Buhake Sindi

Reputation: 89169

The right and wrong approach to writing Java Enums

Recently, I've discovered this code of the following structure:

Interface:

public interface Base<T> {

    public T fromValue(String v);

}

Enum implementation:

public enum AddressType implements Base<AddressType> {

    NotSpecified("Not Specified."),

    Physical("Physical"),

    Postal("Postal");

    private final String label; 

    private AddressType(String label) { 
        this.label = label; 
    } 

    public String getLabel() { 
        return this.label; 
    }

    @Override
    public AddressType fromValue(String v) {
        return valueOf(v);
    }
}

My immediate reaction is that one cannot create an instance of an enum by deserialization or by reflection, so the fromValue() should be static.

I'm not trying to start a debate, but is this correct? I have read, Why would an Enum implement an interface, and I totally agree with the answers provided, but the above example is invalid.

I am doing this because the "architect" doesn't want to take my answer, so this is to create a strong argument (with facts) why the above approach is good/bad.

Upvotes: 0

Views: 1799

Answers (3)

Costi Ciudatu
Costi Ciudatu

Reputation: 38195

Your Base interface seems to serve a whole other purpose (if any).

It is probably meant to be a String-to-T-converter, since it generates a T from a String. The enum is simply wrong if it implements this interface (@yegor256 already pointed out why). So you can keep the enum and you can have some AddressTypeConverter implements Base<AddressType> which calls AddressType.valueOf() in its fromString() method.

But don't get me wrong: enums implementing interfaces are NOT a bad practice, it's just this particular usage that is completely wrong.

Upvotes: 0

yegor256
yegor256

Reputation: 105053

In my opinion this design is wrong. In order to use valueFrom() one has to get an instance of this enum beforehand. Thus, it will look like:

AddressType type = AddressType.Postal.valueFrom("Physical");

What sense does it make?

Upvotes: 1

Marko Topolnik
Marko Topolnik

Reputation: 200148

Your Base interface does not declare valueOf and the fromValue method is indeed implemented. I see no reason why this code should not compile. If you are referring to the valueOf call inside fromValue, that is a call of a static method defined for every enum. I would have to agree, though, that the design of it is quite misguided as you need an arbitrary member of the enum just to call fromValue and get the real member.

On the other hand, in a project that I'm doing right now I have several enums implementing a common interface. This because the enums are related and I want to be able to treat them uniformly with respect to their common semantics.

Upvotes: 1

Related Questions