Multiple If-else or enum - which one is preferable and why?

Question

Here is the original code:

public class FruitGrower {
    public void growAFruit(String type) {
        if ("wtrmln".equals(type)) {
            //do watermelon growing stuff
        } else if ("ppl".equals(type)) {
            //do apple growing stuff
        } else if ("pnppl".equals(type)) {
            //do pineapple growing stuff
        } else if ("rng".equals(type)) {
            //do orange growing stuff
        } else {
            // do other fruit growing stuff
        }
    }
}

This is how I changed it:

public class FruitGrower {
    enum Fruits {
        WATERMELON {
            @Override
            void growAFruit() {
                //do watermelon growing stuff
            }
        },

        APPLE {
            @Override
            void growAFruit() {
                //do apple growing stuff
            }
        },

        PINEAPPLE {
            @Override
            void growAFruit() {
                //do pineapple growing stuff
            }
        },

        ORANGE {
            @Override
            void growAFruit() {
                //do orange growing stuff
            }
        },

        OTHER {
            @Override
            void growAFruit() {
                // do other fruit growing stuff
            }
        };
        static void grow(String type) {
            if ("wtrmln".equals(type)) {
                WATERMELON.growAFruit();
            } else if ("ppl".equals(type)) {
                APPLE.growAFruit();
            } else if ("pnppl".equals(type)) {
                PINEAPPLE.growAFruit();
            } else if ("rng".equals(type)) {
                ORANGE.growAFruit();
            } else {
                OTHER.growAFruit();
            }
        };
        abstract void growAFruit();
    }

    
    public void growAFruit(String type) {
        Fruits.grow(type);
    }
}

I see that enums code is longer and may be not as clear as if-else code, but I believe it's better, could someone tell me, why I'm wrong (or maybe I'm not)?

Are there any concerns about using enum instead of if-else?

Answer 1

You've already got good answers about improving your use of Enums. As for why they're better than string constants:

I think the biggest benefit is compile-time error checking. If I were to call growAFruit("watermelon"), the compiler would have no idea anything was wrong. And since I've spelled it correctly, it's not going to stand out as a mistake when you're viewing the code. But if I were to WATERMELEN.growAFruit(), the compiler can tell you immediately that I've misspelled it.

You also get to define growAFruit as a bunch of simple, easy-to-read methods, instead of a big block of if-then-elses. This becomes even more apparent when you have a few dozen fruits, or when you start adding harvestAFruit(), packageAFruit(), sellAFruit(), etc. Without enums you'd be copying your big if-else block all over, and if you forgot to add a case it would fall through to the default case or do nothing, while with enums the compiler can tell you that the method hasn't been implemented.

Even more compiler-checking goodness: If you also have a growVegetable method and the related string constants, there's nothing stopping you from calling growVegetable("pineapple") or growFruit("peas"). If you've got a "tomato" constant, the only way to know if you consider it a fruit or a vegetable is to read the source code of the relevant methods. Once again, with enums the compiler can tell you right away if you've done something wrong.

Another benefit is that it groups related constants together and gives them a proper home. The alternative being a bunch of public static final fields thrown in some class that happens to use them, or stuck a interface of constants. The interface full of constants doesn't even make sense because if that's all you need defining the enum is much easier than writing out the interface. Also, in classes or interfaces there is the possibility to accidentally use the same value for more than one constant.

They're also iterable. To get all the values of an enum you can just call Fruit.values(), whereas with constants you'd have to create and populate your own array. Or if just using literals as in your example, there is no authoritive list of valid values.

Bonus Round: IDE Support

• With an enum, you can use your IDE's auto-completion feature and automated refactorings
• You can use things like "Find References" in Eclipse with enum values, while you'd have to do a plain text search to find string literals, which will usually also return a lot of false-positives (event if you use static final constants, someone could have used the string literal somewhere)

The main reason not to use an enum would be if you don't know all the possible values at compile time (i.e. you need to add more values while the program is running). In that case you might want to define them as a class heirarchy. Also, don't throw a bunch of unrelated constants into an enum and call it a day. There should be some sort of common thread connecting the values. You can always make multiple enums if that's more appropriate.

Answer 2

Enums are the way to go, but you can dramatically improve your code like this:

public static String grow(String type) {
    return Fruits.valueOf(type.toUpperCase()).gimmeFruit();
};

Oh, you need a default case, that makes it a bit tougher. Of course you can do this:

public static String grow(String type) {
    try{
        return Fruits.valueOf(type.toUpperCase()).gimmeFruit();
    }catch(IllegalArgumentException e){
        return Fruits.OTHER.gimmeFruit();
    }
};

But that's pretty ugly. I guess I'd to something like this:

public static String grow(String type) {
    Fruits /*btw enums should be singular */ fruit = Fruits.OTHER;
    for(Fruits candidate : Fruits.values()){
        if(candidate.name().equalsIgnoreCase(type)){
            fruit = candidate;
            break;
        }
    }
    return fruit.gimmeFruit();
};

Also, if all your enum methods do is return a value, you should refactor your design so that you initialize the values in a constructor and return them in a method defined in the Enum class, not the individual items:

public enum Fruit{
    WATERMELON("watermelon fruit"),
    APPLE("apple fruit")
    // etc.

    ;
    private final String innerName;
    private Fruit(String innerName){ this.innerName = innerName; }
    public String getInnerName(){ return this.innerName; }
}

Answer 3

You have only made half of the changes to be cleaner. The grow method should be changed like this:

static String grow(Fruits type) {
    return type.gimmeFruit();
}

And Fruits should be renamed to Fruit: an apple is a fruit, not a fruits.

If you really need to keep your string types, then define a method (in the enum class itself, for example) returning the Fruit associated to each type. But most of the code should use Fruit instead of String.