Статьи

Синтаксис вкуса: рефакторинг условных выражений

Декабрь 15, 2018

мелодия

Условия по своей природе не плохие, но они, несомненно, имеют неприятную тенденцию к размножению. Подобно грибам, они процветают и размножаются в темных и сырых местах, как, скажем, кодовые базы.

Здесь мы рассмотрим типичное доморощенное условие и посмотрим, как получить небольшую степень контроля.

Удовлетворение противоречивых предпочтений

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

$ fm

Вот и все.

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

 $ fm <GENRE> # e.g. acid-jazz

Наконец, есть режим «О, я не знаю, удиви меня»: 

 $ fm mixtape

mixtape

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

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

О, у нас есть оба вида музыки, кантри и вестерн. –Братья Блюз

Расширение API

Чтобы учесть другие предпочтения, объект Stream 

 class Stream
  def genre_enabled?
    # magic happens here
  end
end

Кодовая база fmConfig 

 Stream.new(Config.new)

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

 # lib/fm/config.rb
module FM
  class Config
    # ...
  end
end

# lib/fm/stream.rb
module FM
  class Stream
    attr_reader :config
    def initialize(config)
      @config = config
    end

    # ...
  end
end

# test/fm/stream_test.rb
gem 'minitest', '~> 5.4'
require 'minitest/autorun'
require './lib/fm/stream'
require './lib/fm/config'

class StreamTest < Minitest::Test
  # ...
end

Тесты можно запустить с помощью следующей команды: 

 ruby test/fm/stream_test.rb

Обеспечение обратной совместимости

Приложение все еще должно работать для тех, кто вообще не хочет настраивать функцию mixtape В этом случае команда mixtape Независимо от того, какой жанр появляется в потоке, он должен быть включен: 

 def test_all_genres_enabled_by_default
  stream = FM::Stream.new(FM::Config.new)

  assert stream.genre_enabled?(:abc), "expected abc to be enabled"
  assert stream.genre_enabled?(:xyz), "expected xyz to be enabled"
end

Тривиальное количество кода проходит тестирование: 

 # lib/fm/stream.rb
def genre_enabled?(genre)
  true
end

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

Выбор пользовательского интерфейса

Довольно идиоматический интерфейс выглядел бы примерно так для жанров белого списка: 

 $ fm configure --only jazz,blues,power-metal,trance

Для менее требовательных слушателей функция внесения в черный список будет выглядеть так: 

 $ fm configure --except house,opera

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

Лучший интерфейс не будет вообще.

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

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

Переменные окружения достаточно просты для работы, и у непрограммистов нет проблем с редактированием текстовых файлов: 

 ENV['FM_GENRE_WHITELIST'] = 'abc,def,ghi,jkl'

Включение и отключение жанров

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

 def test_enabled_via_whitelist
  ENV['FM_GENRE_WHITELIST'] = 'abc, def,ghi , jkl'
  # ...
ensure
  ENV.delete('FM_GENRE_WHITELIST')
end

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

Нам понадобится объект Stream, чтобы сделать всю работу: 

 stream = FM::Stream.new(FM::Config.new)

Наконец, нам понадобятся два типа утверждений:

  1. Все, что определено в списке, включено.

  2. Что-то не в списке выключено. 

     %i(abc def ghi jkl).each do |genre|
  assert stream.genre_enabled?(genre), "expected '#{genre}' to be enabled"
end

refute stream.genre_enabled?(:xyz), "expected xyz to be disabled"

Для прохождения теста не требуется много усилий: 

 # lib/fm/config.rb
def included_genres
  @included_genres ||= ENV['FM_GENRE_WHITELIST'].to_s.split(/\s*,\s*/).map { |s| s.strip.to_sym }
end

# lib/fm/stream.rb
def genre_enabled?(genre)
  return true if config.included_genres.empty?

  config.included_genres.include?(genre)
end

Черный список — это почти то же самое, с перевернутыми утверждениями: 

 def test_disabled_via_blacklist
  ENV['FM_GENRE_BLACKLIST'] = 'abc, def,ghi , jkl'
  stream = FM::Stream.new(FM::Config.new)

  %i(abc def ghi jkl).each do |genre|
    refute stream.genre_enabled?(genre), "expected '#{genre}' to be disabled"
  end

  assert stream.genre_enabled?(:xyz), "expected xyz to be enabled"
ensure
  ENV.delete('FM_GENRE_BLACKLIST')
end

Конфигурация проста: 

 # lib/fm/config.rb
def included_genres
  @included_genres ||= tokenize(ENV['FM_GENRE_WHITELIST'])
end

def excluded_genres
  @excluded_genres ||= tokenize(ENV['FM_GENRE_BLACKLIST'])
end

private

def tokenize(s)
  s.to_s.split(/\s*,\s*/).map { |s| s.strip.to_sym }
end

Объект Stream 

 # lib/fm/stream.rb
def genre_enabled?(genre)
  if config.excluded_genres.empty? && config.included_genres.empty?
    return true
  end
  if !config.included_genres.empty?
    return config.included_genres.include?(genre)
  end
  !config.excluded_genres.include?(genre)
end

Если вы даже слегка булевы, этот код не сразу очевиден.

Удаление ненужной сложности

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

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

Это значительно упрощает вещи: 

 def genre_enabled?(genre)
  if !config.included_genres.empty?
    return config.included_genres.include?(genre)
  end
  !config.excluded_genres.include?(genre)
end

Это не ужасно

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

Обратите внимание на повторяющийся суффикс в именах методов в configincluded_genresexcluded_genres Похоже, что, возможно, мы имеем дело с одним типом вещи, genresincludedexcluded

Или, если хотите, белые и черные списки .

Что если вы всегда знали, что говорите с genres 

 genres.enabled?(:opera)

Это может быть один объект, который оборачивает условное выражение: 

 class Genres
  attr_reader :values
  def initialize(list)
    @values = list.to_s.split(/\s*,\s*/).map { |s| s.strip.to_sym }
  end

  def enabled?(genre)
    if !config.included_genres.empty?
      return config.included_genres.include?(genre)
    end
    !config.excluded_genres.include?(genre)
  end
end

Или у вас может быть два объекта, один для белых списков, а другой для черных списков: 

 class BlacklistedGenres
  # ...
  def enabled?(genre)
    !values.include?(genre.to_sym)
  end
end

class WhitelistedGenres
  # ...
  def enabled?(genre)
    values.include?(genre.to_sym)
  end
end

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

 genres = WhitelistedGenres.new("funk,soul,rock")

Или это может быть 12-летний барабанщик, который живет по соседству и будет слушать что угодно, при условии, что он содержит какой-то ритм: 

 genres = BlacklistedGenres.new("gregorian-chants")

Даже с двумя крошечными объектами условие не исчезло полностью. Это все еще там, в Configтом, как это настроено и с учетом этого конкретного состояния, как мне запросить правильное значение? , вопрос в том, какой объект знает, как решить эту проблему? ,

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

 # lib/fm/config.rb
def genres
  @genres ||= if !!ENV['FM_GENRE_WHITELIST']
    WhitelistedGenres.new(ENV['FM_GENRE_WHITELIST'])
  else
    BlacklistedGenres.new(ENV['FM_GENRE_BLACKLIST'])
  end
end

Как только вы держитесь за экземпляр genres Все, что нужно сделать, это ответить на один простой вопрос: 

 genres.enabled?(:jazz)

Признание потенциального рефакторинга

В книге Мартина Фаулера « Рефакторинг» есть ряд рефакторингов, посвященных превращению условных выражений в объекты. У них причудливые имена, и может показаться, что это все очень продвинутый рефактор-фу.

Это не.

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

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

Это всегда будет правильным выбором? Точно нет. Но сделайте это. Попробуйте это в больших приложениях и небольших приложениях, с большими условиями и небольшими условиями. Почувствуйте, что он может сделать для вас, и где находятся ошибки.

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

Знание того, стоит ли вам рефакторинг вашего if

38
Share: