Несколько If-else или enum - какой из них предпочтительнее и почему? - PullRequest
15 голосов
/ 25 мая 2011

Вот оригинальный код:

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
        }
    }
}

Вот как я это изменил:

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);
    }
}

Я вижу, что код enums длиннее и может быть не таким четким, как код if-else, но я считаю, что лучше, может кто-нибудь сказать мне, почему я ошибаюсь (или, возможно, я не)? 1009 *

UPD - изменен исходный код, чтобы он был более специфичным для проблемы. Я перефразирую вопрос: есть ли опасения по поводу использования enum вместо if-else?

Ответы [ 14 ]

18 голосов
/ 18 июня 2011

У вас уже есть хорошие ответы по улучшению использования Enums. Что касается того, почему они лучше строковых констант:

Я думаю, что наибольшим преимуществом является проверка ошибок во время компиляции . Если бы я позвонил growAFruit("watermelon"), компилятор не знал бы, что что-то не так. И так как я написал это правильно, это не будет выделяться как ошибка при просмотре кода. Но если бы я набрал WATERMELEN.growAFruit(), компилятор мог бы сразу сказать вам, что я его неправильно написал.

Вы также можете определить growAFruit как набор простых, удобных для чтения методов вместо большого блока if - then - else с. Это становится еще более очевидным, когда у вас есть несколько десятков фруктов или когда вы начинаете добавлять harvestAFruit(), packageAFruit(), sellAFruit() и т. Д. Без перечислений вы будете копировать свой большой блок if-else повсюду, а если Вы забыли добавить кейс, он будет падать на кейс по умолчанию или ничего не делать, в то время как с помощью перечислений компилятор может сказать вам, что метод не был реализован.

Еще больше преимуществ проверки компилятором: если у вас также есть метод growVegetable и связанные строковые константы, ничто не помешает вам вызвать growVegetable("pineapple") или growFruit("peas"). Если у вас есть "tomato" константа, единственный способ узнать, считаете ли вы это фруктом или овощем, это прочитать исходный код соответствующих методов. Еще раз, с перечислениями компилятор может сразу сказать вам, если вы сделали что-то не так.

Другое преимущество состоит в том, что он группирует связанные константы вместе и дает им надлежащий дом . В качестве альтернативы можно использовать набор полей public static final, которые выбрасываются в некоторый класс, который использует их, или прикрепляет интерфейс констант. Интерфейс, полный констант, даже не имеет смысла, потому что если это все, что вам нужно, определить перечисление будет намного проще , чем писать интерфейс. Кроме того, в классах или интерфейсах есть возможность случайно использовать одно и то же значение для более чем одной константы.

Они также повторяемые . Чтобы получить все значения перечисления, вы можете просто позвонить Fruit.values(), тогда как с константами вам нужно будет создать и заполнить свой собственный массив. Или, если использовать только литералы, как в вашем примере, нет списка разрешенных допустимых значений.

Бонусный раунд: Поддержка IDE

  • С помощью enum вы можете использовать функцию автозаполнения IDE и автоматический рефакторинг
  • Вы можете использовать такие вещи, как «Найти ссылки» в Eclipse со значениями перечисления, в то время как вам нужно будет выполнить простой текстовый поиск, чтобы найти строковые литералы, которые обычно также возвращают много ложных срабатываний (событие, если вы используете статические конечные константы, кто-то мог где-то использовать строковый литерал)

Основная причина , а не , использовать перечисление, если вы не знаете все возможные значения во время компиляции (т. Е. Вам нужно добавить больше значений во время работы программы). В этом случае вы можете определить их как иерархию классов. Кроме того, не бросайте связку не связанных констант в перечисление и называйте это днем. Должен быть какой-то общий поток, соединяющий значения. Вы всегда можете сделать несколько перечислений, если это более уместно.

17 голосов
/ 25 мая 2011

Перечисления - это путь, но вы можете значительно улучшить свой код следующим образом:

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

О, вам нужен случай по умолчанию, который делает его немного сложнее. Конечно, вы можете сделать это:

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

Но это довольно уродливо. Я думаю, я бы что-то вроде этого:

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();
};

Кроме того, если все ваши методы enum возвращают значение, вы должны реорганизовать свой дизайн, чтобы инициализировать значения в конструкторе и вернуть их в метод, определенный в классе Enum, а не в отдельных элементах:

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; }
}
6 голосов
/ 25 мая 2011

Вы сделали только половину изменений, чтобы быть чище.Метод выращивания должен быть изменен следующим образом:

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

И Fruits следует переименовать в Fruit: яблоко - это фрукт, а не фрукты.

Если вам действительно нужночтобы сохранить ваши строковые типы, затем определите метод (например, в самом классе enum), возвращающий Fruit, связанный с каждым типом.Но большая часть кода должна использовать Fruit вместо String.

4 голосов
/ 16 июня 2011

Я думаю, что вы хотите Map<String, Fruit> (или <String, FruitGrower>).

Эта карта может быть автоматически заполнена конструкторами enum или статическим инициализатором.(Можно даже отобразить несколько имен на одну и ту же константу перечисления, если у некоторых фруктов есть псевдонимы.)

Ваш метод grow будет выглядеть следующим образом:

static void grow(String type) {
   Fruit f = map.get(type);
   if (f == null) {
       OTHER.growFruit();
   }
   else {
       f.growFruit();
   }
}

Конечно, сделайтевам действительно нужна строка здесь?Разве вы не должны всегда использовать объект enum?

3 голосов
/ 22 июня 2011

Я думаю, вы на правильном пути.Я бы пошел и добавил несколько дополнительных байтов для HashMap, чтобы избавиться от блока переключения строк.Это дает вам обоим более чистый внешний вид, меньше кода и, скорее всего, немного дополнительную производительность.

public enum Fruit {

    APPLE("ppl") {
        public void grow() {
            // TODO
        }
    },
    WATERMELON("wtrmln") {
        public void grow() {
            // TODO
        }
    },

    // SNIP extra vitamins go here

    OTHER(null) {
        public void grow() {
            // TODO
        }
    };

    private static Map<String, Fruit> CODE_LOOKUP;
    static {
        // populate code lookup map with all fruits but other
        Map<String, Fruit> map = new HashMap<String, Fruit>();
        for (Fruit v : values()) {
            if (v != OTHER) {
                map.put(v.getCode(), v);
            }
        }
        CODE_LOOKUP = Collections.unmodifiableMap(map);
    }

    public static Fruit getByCode(String code) {
        Fruit f = CODE_LOOKUP.get(code);
        return f == null ? OTHER : f;
    }

    private final String _code;

    private Fruit(String code) {
        _code = code;
    }

    public String getCode() {
        return _code;
    }

    public abstract void grow();
}

И вот как вы его используете:

Fruit.getByCode("wtrmln").grow();

Простой, нет необходимости в FruitGrower, но пойти на это, если вы считаете, что это необходимо.

3 голосов
/ 20 июня 2011

Я не уверен, что использовал бы здесь Enums. Возможно, я что-то здесь упускаю (?), Но я думаю, что мое решение выглядело бы примерно так, с отдельными классами для каждого типа фруктов, все основанные на одном роде:

// Note: Added in response to comment below
public enum FruitType {
    WATERMELON,
    WHATEVERYOUWANT,
    ....
}


public class FruitFactory {

    public Fruit getFruitToGrow(FruitType type) {

        Fruit fruitToGrow = null;

        switch(type){
            case WATERMELON:
                fruitToGrow = new Watermelon();
                break;
            case WHATEVERYOUWANT:
                ...
            default:
                fruitToGrow = new Fruit();
        }

        return fruitToGrow;
    }
}


public class Fruit(){
    public void grow() {
        // Do other fruit growing stuff
    }
}



// Add a separate class for each specific fruit:
public class Watermelon extends Fruit(){
    @override
    public void grow(){
        // Do more specific stuff... 
    }
}
2 голосов
/ 21 июня 2011

Ответом на ваш вопрос является использование перечислений или, что еще лучше, фабрик и полиморфизма, как упоминалось выше. Однако, если вы хотите полностью избавиться от переключателей (что на самом деле делает ваш оператор if-else), хороший способ, если не лучший, сделать это, используя инверсию управления. Таким образом, я предлагаю использовать пружину следующим образом:

public interface Fruit {
   public void grow();
}

public class Watermelon implements Fruit {

   @Override
   public void grow()
   {
       //Add Business Logic Here
   }
}

Теперь создайте интерфейс поиска фруктов следующим образом:

public interface FruitLocator {

Fruit getFruit(String type);
}

Пусть у основного класса есть ссылка на объект FruitLocator, его установщик, и просто вызовите команду getFruit:

private FruitLocator fruitLocator;

public void setFruitLocator (FruitLocator fruitLocator)
{
    this.fruitLocator = fruitLocator;
}

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

Теперь самое сложное. Определите свой класс FruitGrower как боб весны, а также свой FruitLocator и Fruits:

<bean id="fruitGrower" class="yourpackage.FruitGrower">     
    <property name="fruitLocator" ref="fruitLocator" />
</bean>

<bean id="fruitLocator"
    class="org.springframework.beans.factory.config.ServiceLocatorFactoryBean">
    <property name="serviceLocatorInterface"
        value="yourpackage.FruitLocator" />
    <property name="serviceMappings" ref="locatorProperties" />
</bean>
<bean id="locatorProperties"
    class="org.springframework.beans.factory.config.PropertiesFactoryBean">
    <property name="location" value="classpath:fruits.properties" />
</bean>

    <bean id="waterMelon" class="yourpackage.WaterMelon">       
</bean>

Осталось только создать файл fruits.properties в вашем пути к классам и добавить отображение bean-компонента type, как показано ниже:

wtrmln=waterMelon        

Теперь вы можете добавить столько фруктов, сколько пожелаете, вам нужно только создать новый класс фруктов, определить его как bean-компонент и добавить отображение в ваш файл свойств. Гораздо более масштабируемо, чем поиск логики if-else в коде.

Поначалу я знаю, что это кажется сложным, но я думаю, что тема была бы неполной без упоминания Обращения контроля.

2 голосов
/ 21 июня 2011

Публичный API точно такой же. У вас все еще есть тот же блок if-else, он сейчас только в перечислении. Так что я думаю, что это не лучше. Во всяком случае, это хуже из-за дополнительной сложности.

Что значит «заниматься выращиванием фруктов»? Вы говорите о том, что делает плодовод (до почвы, семян и т. Д.), Или о том, что делает сам плод (прорастает, прорастает, цветет и т. Д.)? В исходном примере действия определяются FruitGrower, но в ваших модификациях они определяются Fruit. Это имеет большое значение, когда вы рассматриваете подклассы. Например, я могу определить MasterFruitGrower, который использует разные процессы для лучшего выращивания фруктов. Операция grow(), определенная в Fruit, затрудняет рассуждение.

Насколько сложны операции по выращиванию фруктов? Если вас беспокоит длина строки блока if-else, я думаю, что лучшей идеей будет определить отдельные методы выращивания фруктов (growWatermelon(), growOrange(), ...) или определить интерфейс FruitGrowingProcedure, реализуя подклассы для каждого типа фруктов, и сохраните их в карте или установите под FruitGrower.

2 голосов
/ 20 июня 2011

Я часто реализую метод в перечислениях перечисления данной строки и возвращает соответствующую константу перечисления.Я всегда называю этот метод parse(String).

Иногда я перезагружаю этот метод для того, чтобы анализировать константу перечисления и для другого заданного типа ввода.

Его реализация всегда одинакова: перебирать всеПеречислять значения () и возвращать, когда вы нажмете один.Наконец, сделайте возврат в виде спада - часто это конкретная константа перечисления или null.В большинстве случаев я предпочитаю null.

public class FruitGrower {
    enum Fruit {
        WATERMELON("wtrmln") {
            @Override
            void grow() {
                //do watermelon growing stuff
            }
        },

        APPLE("ppl") {
            @Override
            void grow() {
                //do apple growing stuff
            }
        },

        PINEAPPLE("pnppl") {
            @Override
            void grow() {
                //do pineapple growing stuff
            }
        },

        ORANGE("rng") {
            @Override
            void grow() {
                //do orange growing stuff
            }
        },

        OTHER("") {
            @Override
            void grow() {
                // do other fruit growing stuff
            }
        };

        private String name;

        private Fruit(String name) {
            this.name = name;
        }

        abstract void grow();

        public static Fruit parse(String name) {
            for(Fruit f : values()) {
                if(f.name.equals(name)){
                    return f;
                }
            }

            return OTHER; //fallthrough value (null or specific enum constant like here.)
        }
    }


    public void growAFruit(String name) {
        Fruit.parse(name).grow();
    }
}

Если вам не нужен Fruit.OTHER, удалите его.Или как растет «Другой фрукт»?oO Вернуть null затем в метод parse(String) в качестве значения по умолчанию и выполнить нулевую проверку перед вызовом grow() в growAFruit(String).

Хорошей идеей будет добавить аннотацию @CheckForNull к parse(String) метод тогда.

2 голосов
/ 25 мая 2011

Вы также можете улучшить его, используя переменную для хранения значения gimmeFruit и инициализации с помощью конструктора.

(я на самом деле не скомпилировал это, поэтому могут быть некоторые синтаксические ошибки)

public class FruitGrower {
    enum Fruits {
        WATERMELON("watermelon fruit"),
        APPLE("apple fruit"),
        PINEAPPLE("pineapple fruit"),
        ORANGE("orange fruit"),
        OTHER("other fruit")

        private String gimmeStr;

        private Fruits(String gimmeText) {
            gimmeStr = gimmeText;
        }

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

        public String gimmeFruit(String type) {
            return gimmeStr;
        }

    }
}

EDIT: Если тип для метода Grow не совпадает со строкой, используйте Map для определения совпадений типа с Enum и возврата поиска с карты.

...