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

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

-Rick

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

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

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

http: // elegancecode .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;
    }
}

Этот оператор switch в порядке. Ребята, у вас нет других ошибок, на которые нужно обратить внимание? lol

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

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

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

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

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

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

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

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

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

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

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

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

I wouldn't use an if. An if would be less clear than the switch. The switch is telling me that you are comparing the same thing throughout.

Just to scare people, this is less clear than your code:

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;

This switch may be OK, you might want to look at @Talljoe suggestion.

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

Если в вашей программе много специфических скидок, вы можете провести рефакторинг, например:

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

Тогда объект скидки будет содержать все связанные атрибуты и методы. для выяснения скидок.

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

Словарь TallJoe - хороший подход, однако.

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

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

тогда

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

будет достаточно.

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

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, и переместил функцию.

Да, это похоже на правильное использование оператора switch .

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

Почему вы не включили метку по умолчанию? Добавление исключения в метку по умолчанию гарантирует, что программа завершится ошибкой, если вы добавите новый 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.");
}

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

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

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

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

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

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