Когда нужно попытаться исключить оператор switch? [Дубликат]

На этот вопрос уже есть ответ:

Переключатель (случай) всегда не так? 8 ответов

Я столкнулся с оператором switch в кодовой базе, над которой я работаю, и пытаюсь выяснить, как заменить его чем-то лучшим, поскольку заявления переключателя считаются запахом кода. Однако, прочитавнескольк сообщений на стеке потока о Замена Переключатель заявлени Кажется, я не могу придумать эффективный способ заменить этот конкретный оператор switch.

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

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

private MyType DoSomething(IDataRecord reader)
{
    var p = new MyType
                {
                   Id = (int)reader[idIndex],
                   Name = (string)reader[nameIndex]
                }

    switch ((string) reader[discountTypeIndex])
    {
        case "A":
            p.DiscountType = DiscountType.Discountable;
            break;
        case "B":
            p.DiscountType = DiscountType.Loss;
            break;
        case "O":
            p.DiscountType = DiscountType.Other;
            break;
    }

    return p;
}

Кто-нибудь может предложить способ устранить этот переключатель? Или это правильное использование переключателя? И если это так, существуют ли другие подходящие варианты использования операторов switch? Мне бы очень хотелось знать, где они уместны, поэтому я не трачу слишком много времени, пытаясь исключить каждое высказывание переключателя, с которым я сталкиваюсь, только потому, что в некоторых обстоятельствах их считают запахо

Обновить По предложениюМайк Я немного искал дублирование этой логики и обнаружил, что кто-то создал логику в другом классе, что фактически сделало весь оператор switch избыточным. Таким образом, в контексте этого конкретного фрагмента кода оператор switch был ненужным. Тем не менее, мой вопрос больше касается уместности операторов switch в коде и того, должны ли мы всегда пытаться заменить их всякий раз, когда они найдены, поэтому в этом случае я склонен принять ответ, что этот оператор switch уместен.

Ответы на вопрос(13)

что они являются операторами переключения, а потому, что их присутствие предполагает необходимость рефакторинга.

Проблема с операторами switch заключается в том, что они создают раздвоение в вашем коде (точно так же, как оператор if). Каждая ветвь должна быть проверена индивидуально, и каждая ветвь в каждой ветке и ... ну, вы поняли.

Тем не менее, в следующей статье есть несколько полезных советов по использованию операторов switch:

http: //elegantcode.com/2009/01/10/refactoring-a-switch-statement

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

private DiscountType GetDiscountType(string discount)
{
    switch (discount)
    {
        case "A": return DiscountType.Discountable;
        case "B": return DiscountType.Loss;
        case "O": return DiscountType.Other;
    }
}

if. Анif будет менее понятным, чемswitch.switch говорит мне, что вы сравниваете одно и то же.

Просто чтобы напугать людей, это менее понятно, чем твой код:

if (string) reader[discountTypeIndex]) == "A")
   p.DiscountType = DiscountType.Discountable;
else if (string) reader[discountTypeIndex]) == "B")
   p.DiscountType = DiscountType.Loss;
else if (string) reader[discountTypeIndex]) == "O")
   p.DiscountType = DiscountType.Other;

Этоswitch может быть хорошо, вы можете посмотреть на предложение @Talljoe.

ребята, нет других ошибок, о которых можно было бы рассказать? смешн

Однако есть одна вещь, которую я заметил ... Вы не должны использовать порядковые номера индексов в индексаторе объектов IReader [] .... что делать, если порядок столбцов изменяется? Попробуйте использовать имена полей, т.е. reader ["id"] и reader ["name"]

Однако у меня есть еще один вопрос к тебе.

Почему вы не включили метку по умолчанию? Если вы добавите исключение в метку по умолчанию, программа будет работать правильно, когда вы добавите новый discountTypeIndex и забудете изменить код.

Также, если вы хотите отобразить строковое значение в Enum, вы можете использовать Атрибуты и отражение.

Что-то типа

public enum DiscountType
{
    None,

    [Description("A")]
    Discountable,

    [Description("B")]
    Loss,

    [Description("O")]
    Other
}

public GetDiscountType(string discountTypeIndex)
{
    foreach(DiscountType type in Enum.GetValues(typeof(DiscountType))
    {
        //Implementing GetDescription should be easy. Search on Google.
        if(string.compare(discountTypeIndex, GetDescription(type))==0)
            return type;
    }

    throw new ArgumentException("DiscountTypeIndex " + discountTypeIndex + " is not valid.");
}

но в случае, если вы представите, я бы, по крайней мере, исключил дублирование назначения DiscountType; Я мог бы вместо этого написать функцию, которая возвращает DiscountType с заданной строкой. Эта функция могла бы просто иметь операторы return для каждого случая, устраняя необходимость в разрыве. Я считаю, что перерывы между делами переключения очень коварны.

private MyType DoSomething(IDataRecord reader)
{
    var p = new MyType
                {
                   Id = (int)reader[idIndex],
                   Name = (string)reader[nameIndex]
                }

    p.DiscountType = FindDiscountType(reader[discountTypeIndex]);

    return p;
}

private DiscountType FindDiscountType (string key) {
    switch ((string) reader[discountTypeIndex])
    {
        case "A":
            return DiscountType.Discountable;
        case "B":
            return DiscountType.Loss;
        case "O":
            return DiscountType.Other;
    }
    // handle the default case as appropriate
}

Довольно скоро, я бы заметил, что FindDiscountType () действительно принадлежит классу DiscountType и переместил функцию.

запахи не в выражениях, а в том, что внутри. Это утверждение переключателя в порядке, пока он не начнет добавлять еще пару случаев. Тогда, возможно, стоит создать справочную таблицу:

private static Dictionary<string, DiscountType> DiscountTypeLookup = 
  new Dictionary<string, DiscountType>(StringComparer.Ordinal)
    {
      {"A", DiscountType.Discountable},
      {"B", DiscountType.Loss},
      {"O", DiscountType.Other},
    };

В зависимости от вашей точки зрения, это может быть более или менее читабельным.

Где все начинает пахнуть, если содержание вашего дела превышает одну или две строки.

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

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

p.Discount = DiscountFactory.Create(reader[discountTypeIndex]);

Затем объект скидки содержит все атрибуты и методы, связанные с определением скидок.

так как она делает выбор читабельным, а также легко добавляется или вычитается.

Смотрите эту ссылку.

наконец, у вас есть возможность удалить самый уродливый, неинтуитивный, подверженный ошибкам синтаксис во всем языке.

Это когда вы пытаетесь удалить оператор switch.

Просто чтобы быть ясно, я имею в виду синтаксис. Это то, что взято из C / C ++, и его следовало изменить, чтобы он соответствовал более современному синтаксису в C #. Я полностью согласен с концепцией предоставления переключателя, чтобы компилятор мог оптимизировать переход.

что изменение кода ради изменения кода не лучшее использование времени. Изменение кода, чтобы сделать его [более читабельным, быстрым, более эффективным и т. Д. И т. Д.], Имеет смысл. Не меняйте его только потому, что кто-то говорит, что вы делаете что-то «вонючее».

-Rick

авление кода символа с перечисляемым значением. Лучше всего это выражать как отображение, где детали отображения предоставляются в одном месте, либо на карте (как предполагает Таллджо), либо в функции, которая использует оператор switch (как предложено Робертом Харви).

Оба эти метода, вероятно, хороши в этом случае, но я хотел бы обратить ваше внимание на принцип дизайна, который может быть полезен здесь или в других подобных случаях. Открытый / Закрытый руководитель:

http: //en.wikipedia.org/wiki/Open/closed_principlhttp: //www.objectmentor.com/resources/articles/ocp.pd (обязательно прочитайте!)

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

Я думаю, что это хорошая статья о шаблоне реестра. Посмотрите, как в реестре содержится сопоставление некоторого ключа с некоторым значением? Таким образом, это похоже на ваше отображение, выраженное как оператор switch. Конечно, в вашем случае вы не будете регистрировать объекты, которые все реализуют общий интерфейс, но вы должны получить суть:

http: //sinnema313.wordpress.com/2009/03/01/the-registry-pattern

Итак, чтобы ответить на исходный вопрос - оператор case является плохой формой, так как я ожидаю, что преобразование из кода символа в перечисляемое значение будет необходимо в нескольких местах в вашем приложении, поэтому оно должно быть учтено. Два ответа, на которые я ссылался, дают вам хороший совет о том, как это сделать - сделайте свой выбор, какой вы предпочитаете. Однако, если сопоставление может со временем измениться, рассмотрите шаблон реестра как способ изолировать ваш код от последствий таких изменений.

любой оператор switch, который зависит от типа чего-либо, может указывать на отсутствующий полиморфизм (или на отсутствующие подклассы).

ем не менее, хороший словарь @ TallJoe.

Обратите внимание, чтоесл ваши перечисления и значения базы данных были целыми числами, а не строками, или если значения вашей базы данных совпадали с именами перечислений, то сработало бы отражение, например, данны

public enum DiscountType : int
{
    Unknown = 0,
    Discountable = 1,
    Loss = 2,
    Other = 3
}

тогд

p.DiscountType = Enum.Parse(typeof(DiscountType), 
    (string)reader[discountTypeIndex]));

хватит.

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

Я бы попытался получить создание MTypes в одном месте в вашей программе, возможно, в конструкторе самого MType или в каком-то заводском методе, но наличие случайных частей вашей программы, присваивающих значения, может привести к тому, что кто-то не будет знать, как эти значения должно быть и делать что-то не так.

Так что переключатель хорош, но, возможно, его нужно больше перемещать внутри части создания вашего Типа

ВАШ ОТВЕТ НА ВОПРОС