Когда перечисления НЕ являются запахом кода?

17

дилемма

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

Поэтому я ищу руководящие принципы и / или варианты использования, в которых перечисления НЕ являются запахом кода и фактически являются допустимой конструкцией.

Источники:

«ПРЕДУПРЕЖДЕНИЕ. Как правило, перечисления являются запахами кода и должны быть преобразованы в полиморфные классы. [8]» Seemann, Mark, Dependency Injection, .Net, 2011, p. 342

[8] Мартин Фаулер и др., Рефакторинг: Улучшение дизайна существующего кода (Нью-Йорк: Аддисон-Уэсли, 1999), 82.

контекст

Причиной моей дилеммы является торговый API. Они дают мне поток данных о тиках, отправляя этот метод:

void TickPrice(TickType tickType, double value)

где enum TickType { BuyPrice, BuyQuantity, LastPrice, LastQuantity, ... }

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

Dictionary<TickType,double> LastValues

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

Легко найти DON'Ts перечислений, но не так просто, и я был бы признателен, если бы люди могли поделиться своим опытом, плюсами и минусами.

Второстепенные мысли

Некоторые решения и действия основаны на них, TickTypeи я не могу придумать способ устранить операторы enum / switch. Самым чистым решением, которое я могу придумать, является использование фабрики и возвращение реализации на основе TickType. Даже тогда у меня все еще будет оператор switch, который возвращает реализацию интерфейса.

Ниже приведен один из примеров классов, где у меня есть сомнения, что я могу использовать неправильное перечисление:

public class ExecutionSimulator
{
  Dictionary<TickType, double> LastReceived;
  void ProcessTick(TickType tickType, double value)
  {
    //Store Last Received TickType value
    LastReceived[tickType] = value;

    //Perform Order matching only on specific TickTypes
    switch(tickType)
    {
      case BidPrice:
      case BidSize:
        MatchSellOrders();
        break;
      case AskPrice:
      case AskSize:
        MatchBuyOrders();
        break;
    }       
  }
}
stromms
источник
26
Я никогда не слышал, чтобы перечисления были запахом кода. Не могли бы вы включить ссылку? Я думаю, что они имеют огромное значение для ограниченного числа потенциальных ценностей
Ричард Тингл
25
Какие книги говорят, что перечисления являются запахом кода? Получить лучшие книги.
JacquesB
3
Таким образом, ответ на вопрос будет «большую часть времени».
gnasher729
5
Хорошее, безопасное значение типа, которое передает значение? Это гарантирует, что правильные ключи используются в словаре? Когда это запах кода?
7
Без контекста я думаю, что лучшая формулировка была быenums as switch statements might be a code smell ...
pllee

Ответы:

31

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

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

Карл Билефельдт
источник
3
Это самый важный момент - добавление дополнительного значения к перечислению означает поиск каждого использования типа в вашем коде и, возможно, необходимость добавления новой ветви для нового значения. Сравните с полиморфным типом, где добавление новой возможности просто означает создание нового класса, который реализует интерфейс - изменения сосредоточены в одном месте, поэтому их легче сделать. Если вы уверены, что новый тип тиков никогда не будет добавлен, перечисление в порядке, иначе оно должно быть заключено в полиморфный интерфейс.
Жюль
Это один из ответов, которые я искал. Я просто не могу избежать этого, когда метод, который я упаковываю, использует перечисление, и цель состоит в том, чтобы централизовать это перечисление в одном месте. Я должен сделать оболочку такой, чтобы, если API изменял эти перечисления, мне нужно было только менять оболочку, а не ее потребителей.
штурмы
@Jules - «Если вы уверены, что новый тип тиков никогда не будет добавлен ...» Это утверждение неверно на многих уровнях. Класс для каждого отдельного типа, несмотря на то, что каждый тип имеет одинаковое поведение, настолько далек от «разумного» подхода, насколько это возможно. Это происходит до тех пор, пока вы не будете вести себя по-разному и не начнете нуждаться в выражениях «switch / if-then», где нужно провести линию. Это, конечно, не зависит от того, смогу ли я добавить дополнительные значения enum в будущем.
Данк
@ Думаю, вы не поняли мой комментарий. Я никогда не предлагал создавать несколько типов с одинаковым поведением - это было бы сумасшествием.
Жюль
1
Даже если вы «перечислили все возможные значения», это не означает, что тип никогда не будет иметь дополнительных функций для каждого экземпляра. Даже что-то вроде Month может иметь другие полезные свойства, например количество дней. Использование enum немедленно предотвращает расширение концепции, которую вы моделируете. Это в основном механизм / паттерн против ООП.
Дейв Кузино
17

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

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

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

Уинстон Эверт
источник
Вы говорите, что когда перечисления содержат существительные в качестве записей (например, цвета), они, вероятно, используются правильно. И когда они являются глаголами (подключено, рендеринг), то это признак того, что они используются неправильно?
штормы
2
@stromms, грамматика - ужасное руководство по архитектуре программного обеспечения. Если бы TickType был интерфейсом, а не перечислением, какими были бы методы? Я не вижу ничего, поэтому мне кажется, что это перечисление. В других случаях, таких как состояние, цвета, серверная часть и т. Д., Я могу придумать методы, и поэтому они являются интерфейсами.
Уинстон Эверт
@WinstonEwert и редактирование, кажется , что они являются различными типами поведения.
Оооо. Так что, если я могу придумать метод для перечисления, то это может быть кандидатом на интерфейс? Это может быть очень хорошим моментом.
штурмы
1
@stromms, не могли бы вы поделиться большим количеством примеров переключателей, чтобы я мог лучше понять, как вы используете TickType?
Уинстон Эверт
8

При передаче данных перечисления не имеют запаха кода

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

Считаю предпочтительным передачу произвольной stringsили ints. Строки могут вызывать проблемы из-за изменений в написании и заглавных буквах. Ints позволяют передавать из диапазона значений и имеют мало семантики (например , я получаю 3от вашего торгового обслуживания, что же это значит? LastPrice? LastQuantity? Что - то еще?

Передача объектов и использование иерархий классов не всегда возможны; например, не позволяет принимающей стороне различать, какой класс был отправлен.

В моем проекте служба использует иерархию классов для воздействия операций, непосредственно перед передачей через DataContractобъект объект из иерархии классов копируется в unionподобный объект, который содержит enumтип для указания типа. Клиент получает DataContractи создает объект класса в иерархии, используя enumзначение для switchсоздания объекта правильного типа.

Другая причина, по которой не нужно передавать объекты класса, заключается в том, что для службы может потребоваться совершенно другое поведение для переданного объекта (такого как LastPrice), чем для клиента. В этом случае отправка класса и его методов нежелательна.

Являются ли заявления о переключении плохими?

ИМХО, единственное switch утверждение, которое вызывает различные конструкторы в зависимости от, enumне является запахом кода. Это не обязательно лучше или хуже, чем другие методы, такие как рефлексия на имя типа; это зависит от реальной ситуации.

Включение enumповсюду - это запах кода, предлагает альтернативы, которые зачастую лучше:

  • Использовать объекты разных классов иерархии, которые имеют переопределенные методы; т.е. полиморфизм.
  • Используйте шаблон посетителя, когда классы в иерархии редко изменяются и когда (многие) операции должны быть слабо связаны с классами в иерархии.
Каспер ван ден Берг
источник
На самом деле, TickType передается через провод в виде int. Моя обертка - это та, которая использует, TickTypeотлитая из полученного int. Несколько событий используют этот тип тика с дикими переменными сигнатурами, которые являются ответами на различные запросы. Является ли обычной практикой постоянное использование intразличных функций? Например, TickPrice(int type, double value)использует 1,3 и 6, в typeто время как TickSize(int type, double valueиспользует 2,4, и 5? Имеет ли смысл разделять их на два события?
штурмы
2

Что делать, если вы пошли с более сложным типом:

    abstract class TickType
    {
      public abstract string Name {get;}
      public abstract double TickValue {get;}
    }

    class BuyPrice : TickType
    {
      public override string Name { get { return "Buy Price"; } }
      public override double TickValue { get { return 2.35d; } }
    }

    class BuyQuantity : TickType
    {
      public override string Name { get { return "Buy Quantity"; } }
      public override double TickValue { get { return 4.55d; } }
    }
//etcetera

тогда вы можете загрузить свои типы из рефлексии или построить его самостоятельно, но главное, что вы здесь делаете, это то, что вы придерживаетесь принципа Открытого Закрытия SOLID

Крис
источник
1

TL; DR

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

Прошу прощения, как я рефакторинг тонн моего старого кода. / вздох; ^ D


Но почему?

Довольно хороший обзор, почему это так от LosTechies здесь :

// calculate the service fee
public double CalculateServiceFeeUsingEnum(Account acct)
{
    double totalFee = 0;
    foreach (var service in acct.ServiceEnums) { 

        switch (service)
        {
            case ServiceTypeEnum.ServiceA:
                totalFee += acct.NumOfUsers * 5;
                break;
            case ServiceTypeEnum.ServiceB:
                totalFee += 10;
                break;
        }
    }
    return totalFee;
} 

Это имеет все те же проблемы, что и код выше. По мере того как приложение становится больше, шансы иметь похожие операторы ветвления будут увеличиваться. Кроме того, по мере развертывания дополнительных услуг премиум-класса вам придется постоянно изменять этот код, что нарушает принцип Open-Closed. Здесь есть и другие проблемы. Функция для расчета платы за услуги не должна знать фактические суммы каждой услуги. Это информация, которая должна быть заключена в капсулу.

Небольшое отступление: перечисления - очень ограниченная структура данных. Если вы не используете enum для обозначения целого числа, обозначенного как целое число, вам нужен класс для правильного моделирования абстракции. Вы можете использовать удивительный класс Enumeration Джимми, чтобы использовать классы и использовать их в качестве меток.

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

public interface ICalculateServiceFee
{
    double CalculateServiceFee(Account acct);
}

...

Теперь мы можем создать наши конкретные реализации интерфейса и прикрепить их [к] учетной записи.

public class Account{
    public int NumOfUsers{get;set;}
    public ICalculateServiceFee[] Services { get; set; }
} 

public class ServiceA : ICalculateServiceFee
{
    double feePerUser = 5; 

    public double CalculateServiceFee(Account acct)
    {
        return acct.NumOfUsers * feePerUser;
    }
} 

public class ServiceB : ICalculateServiceFee
{
    double serviceFee = 10;
    public double CalculateServiceFee(Account acct)
    {
        return serviceFee;
    }
} 

Еще один случай реализации ...

Суть в том, что если у вас есть поведение, которое зависит от значений перечисления, почему бы вместо этого не иметь различные реализации аналогичного интерфейса или родительского класса, который гарантирует, что значение существует? В моем случае я смотрю различные сообщения об ошибках, основанные на кодах состояния REST. Вместо того...

private static string _getErrorCKey(int statusCode)
{
    string ret;

    switch (statusCode)
    {
        case StatusCodes.Status403Forbidden:
            ret = "BRANCH_UNAUTHORIZED";
            break;

        case StatusCodes.Status422UnprocessableEntity:
            ret = "BRANCH_NOT_FOUND";
            break;

        default:
            ret = "BRANCH_GENERIC_ERROR";
            break;
    }

    return ret;
}

... возможно, я должен обернуть коды статуса в классах.

public interface IAmStatusResult
{
    int StatusResult { get; }    // Pretend an int's okay for now.
    string ErrorKey { get; }
}

Затем каждый раз, когда мне нужен новый тип IAmStatusResult, я его кодирую ...

public class UnauthorizedBranchStatusResult : IAmStatusResult
{
    public int StatusResult => 403;
    public string ErrorKey => "BRANCH_UNAUTHORIZED";
}

... и теперь я могу убедиться, что более ранний код понимает, что он имеет IAmStatusResultобласть действия, и ссылаться на него, entity.ErrorKeyа не на более замысловатый deadend_getErrorCode(403) .

И, что более важно, каждый раз , когда я добавляю новый тип возвращаемого значения, не нужно добавлять никакого другого кода для его обработки . Хоть знаешь, то enumи switchбыли кодовые запахи.

Прибыль.

Ruffin
источник
Как насчет случая, когда, например, у меня есть полиморфные классы, и я хочу настроить, какой из них я использую, с помощью параметра командной строки? Командная строка -> enum -> switch / map -> реализация кажется довольно законным вариантом использования. Я думаю, что ключевым моментом является то, что enum используется для оркестровки, а не для реализации условной логики.
Муравей Р
@AntP Почему бы, в этом случае, не использовать StatusResultзначение? Вы могли бы утверждать, что перечисление полезно в качестве прецедента, запоминающегося человеком, в этом случае использования, но я, вероятно, все еще назвал бы это запахом кода, поскольку есть хорошие альтернативы, которые не требуют закрытой коллекции.
ruffin
Какие альтернативы? Строка? Это произвольное различие с точки зрения «перечисления должны быть заменены полиморфизмом» - любой подход требует отображения значения для типа; избегая перечисления в этом случае ничего не добивается.
Муравей P
@AntP Я не уверен, где провода пересекаются здесь. Если вы ссылаетесь на свойство в интерфейсе, вы можете использовать этот интерфейс в любом месте, где это необходимо, и никогда не обновлять этот код, когда создаете новую (или удаляете существующую) реализацию этого интерфейса . Если у вас есть enum, вы действительно должны кода обновления каждый раз , когда вы добавляете или удаляете значение, и потенциально много мест, в зависимости от того, как структурирована вашего кода. Использование полиморфизма вместо перечисления - это то же самое, что гарантировать, что ваш код находится в нормальной форме Бойса-Кодда .
ruffin
Да. Теперь предположим, что у вас есть N таких полиморфных реализаций. В статье Los Techies они просто перебирают все из них. Но что, если я хочу условно применить только одну реализацию, основанную, например, на параметрах командной строки? Теперь мы должны определить отображение из некоторого значения конфигурации в некоторый тип реализации для инъекций. Перечисление в этом случае так же хорошо, как и любой другой тип конфигурации. Это пример ситуации, когда больше нет возможности заменить «грязное перечисление» полиморфизмом. Ветвь по абстракции может только увести вас так далеко.
Муравей P
0

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

  • Организовать код в соответствии с операциями. В этом случае вы можете использовать перечисление для тегирования разных типов и иметь оператор switch в каждой процедуре, которая использует тегированные данные.
  • Организовать код в соответствии с типами данных. В этом случае вы можете заменить перечисление интерфейсом и использовать класс для каждого элемента перечисления. Затем вы реализуете каждую операцию как метод в каждом классе.

Какое решение лучше?

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

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

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

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

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

Итог: перечисления не обязательно являются запахом кода.

Джорджио
источник
обратите внимание, что вопрос был задан в контексте C #, языка ООП. также интересно, что группировка по операции или по типу - это две стороны одной медали, но я не вижу, что одна «лучше», чем другая, как вы описываете. результирующий объем кода одинаков, а языки ООП уже облегчают организацию по типам. это также производит значительно более интуитивный (легкий для чтения) код.
Дэйв Кузино
@DaveCousineau: «Я не считаю, что одно« лучше », чем другое, как вы описываете»: я не говорил, что одно всегда лучше другого. Я сказал, что одно лучше другого в зависимости от контекста. Например , если операции более или менее фиксированными и вы планируете добавление новых типов (например, графический интерфейс с фиксированной paint(), show(), close() resize()операции и пользовательские виджеты) , то объектно-ориентированный подход лучше в том смысле , что позволяет добавить новый тип , не затрагивая слишком много существующего кода (вы в основном реализуете новый класс, который является локальным изменением).
Джорджио
Я также не вижу «лучше», как вы описали, что означает, что я не вижу, что это зависит от ситуации. Количество кода, которое вы должны написать, одинаково в обоих сценариях. Разница лишь в том, что один путь противоположен ООП (перечисления), а другой - нет.
Дэйв Кузино
@DaveCousineau: количество кода, которое вы должны написать, вероятно, одинаково, местность (места в исходном коде, который вы должны изменить и перекомпилировать) резко меняется. Например, в ООП, если вы добавляете новый метод в интерфейс, вы должны добавить реализацию для каждого класса, который реализует этот интерфейс.
Джорджио
Разве это не вопрос глубины VS глубины? Добавление 1 метода в 10 файлов или 10 методов в 1 файл.
Дейв Кузино