вторник, 30 августа 2016 г.

Что не так с оператором switch?

В обсуждении одного из моих ответов на ru.stackoverflow в G+ был поднят вопрос по поводу того, является ли оператор switch design или code smell-ом?

Тут нужно быстро вспомнить, откуда ноги вообще растут (ИМХО). В наших с вами разных языках программирования существует много разных способов решения одной и той же проблемы. Например, когда у нас есть определенная задача (нарисовать фигуру?!) и несколько разновидностей входных данных (круг, квадрат, прямоугольник?), то решить ее можно разными способами. Можно взять впихнуть тип в структурку и в методе draw перебрать все возможные варианты.

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

Чтобы следовать чьим-то советом правильно, а не как обычно, нужно понимать контекст, в котором этот самый совет дается.

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

Но даже в этом случае нужно понимать, на какой компромисс мы идем: иерархия наследования позволяет легко добавить новый тип фигуры (минимум изменений), но усложняет добавление новых операций (много изменений во множестве типов). В этом плане подход на основе объектов упрощает изменение системы в одну сторону, а тот же структурный или функциональный подход – в другую (в этом случае проще будет добавлять именно новую операцию, а не новый тип). Эта проблема широко известна под названием Expression Problem, а сравнение ФП и ОО подходов я делал в статье OCP: ФП vs. ООП.

Ну, ок, с фигурками мы разобрались, а как насчет других случаев использования оператора switch? Ведь он не расширяемый и нам, о ужас, придется изменять код при изменении требований?

Тут, как всегда, все зависит от контекста, решаемой задачи и того, что в этом switch-е происходит.

Вот, например, если у вас есть фабрика, которая создает экземпляр некоторого объекта по входным данным, например, некоторый парсер в зависимости от расширения файла:

public class ParserFactory
{
   
public static IParser CreateParser(string
fileName)
    {
       
switch (Path.
GetExtension(fileName))
        {
           
case ".txt"
:
               
return new TextParser
();
           
case ".xml"
:
               
return new XmlParser
();
           
default
:
               
throw new NotSupportedException();
        }
    }
}

Кто-то скажет, что решение #нерасширябельное, нарушает SRP и вообще, никуда не годится.

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

Но просто подумайте, а какие здесь альтернативы: прикрутить полиморфизм? Фабрику? А кто будет создавать фабрику? Что, MEF прикручивать и саморегистриуемые компоненты писать? Да, а крыша не поеедет это все сопровождать? Да, можно прикрутить словарик, но даже в этом случае он уместен, когда кода на каждый кейс много или самих кейсов сильно больше 2-х или 3-х.

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

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

16 комментариев:

  1. Городить иерархию классов вместо switch, это видимо кого-то книги "Мартина" слишком сильно затянули, безвозвратно. Ещё вспоминается байка про Фаулера, Бека и рефакторинг "Замена паттерна Стратегия оператором if" :)

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

    ОтветитьУдалить
    Ответы
    1. Вам это байка, а вот у человека это суровая реальность - http://www.yegor256.com/2015/08/18/multiple-return-statements-in-oop.html

      Удалить
  2. >Если дописываю или начинает более чем в одном месте использоваться - по ситуации - обычно либо Dictionary, либо "стратегии".

    Либо, наверное, вынес switch-а в отдельный метод?

    ОтветитьУдалить
    Ответы
    1. Тоже вариант. Правда всё равно туда приедут в параметр лямбды или что-то подобное (раз уж switch у нас не в одном месте, значит должна работать разная логика), а как только с этим связываешься, задумываешься про Dictionary. По крайней мере у меня так :)

      Удалить
  3. Полностью согласен с авторами ответов. У автора на stackoverflow изначально неверные требования, к расширению. Если речь в расширении идет всего лишь о методе, то это слишком узкий взгляд и надо либо смотреть шире либо думать о расширении, когда в этом будет прямая необходимость. А так это пустая трата мыслей и времени.

    ОтветитьУдалить
  4. А вот как быть со свитчом с паттерн матчингом?

    ОтветитьУдалить
    Ответы
    1. Меня в нынешних switch-ах очень смущает необходимость писать default реализацию, по той причине, что если я добавляю какую-либо ветку (например, новое значение энума), то я вынужден сам шерстить код в поиске мест где надо добавить логику.
      К сожалению, в новой версии шарпа паттерн-матчинг "кривоват", в плане того что алгебраических типов данных (discriminated union-ов) там не будет. И по прежнему надо реализовывать default ветку.
      Т.е. никакой помощи от компилятора ждать не приходится :(

      Удалить
    2. ВОт это мне всегда нравится :) т.е. новое значение добавляем, а просмотреть что делает код по старым значениям не, не надо?

      Удалить
    3. вообще я думаю в идеале этого хотелось бы избегать

      Удалить
  5. Этот комментарий был удален автором.

    ОтветитьУдалить
  6. Мартин всё ж таки не даёт тебе покоя. Троллишь его через раз :)

    ОтветитьУдалить
  7. "а крыша не поеедет"
    опечатка

    ОтветитьУдалить