Источник Антипаттерны, связанные с кодом

В данном документе описаны потенциальные антипаттерны, связанные с кодом, конкретными приемами Elixir и его функциями.

Чрезмерное использование комментариев

Проблема

Чрезмерное использование комментариев или добавление комментариев к уже очевидному коду может привести к ухудшению читаемости кода.

Пример

# Returns the Unix timestamp of 5 minutes from the current time
defp unix_five_min_from_now do
  # Get the current time
  now = DateTime.utc_now()

  # Convert it to a Unix timestamp
  unix_now = DateTime.to_unix(now, :second)

  # Add five minutes in seconds
  unix_now + (60 * 5)
end

Переработка

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

Вы можете переработать код следующим образом:

@five_min_in_seconds 60 * 5

defp unix_five_min_from_now do
  now = DateTime.utc_now()
  unix_now = DateTime.to_unix(now, :second)
  unix_now + @five_min_in_seconds
end

Мы удалили ненужные комментарии. Также мы добавили атрибут модуля @five_min_in_seconds, который служит дополнительной целью придания имени «волшебному» числу 60 * 5, делая код более понятным и выразительным.

Дополнительные замечания

Elixir чётко различает документацию и комментарии к коду. Язык имеет встроенную поддержку документации на основе @doc, @moduledoc и других механизмов. Дополнительную информацию можно найти в руководстве "Написание документации".

Сложные else-блоки в with

Проблема

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

Пример

Пример такого антипаттерна, как показано ниже, представляет собой функцию open_decoded_file/1, которая считывает содержимое строки, закодированной в Base64, из файла и возвращает декодированную двоичную строку. Эта функция использует with-выражение, которое должно обрабатывать две возможные ошибки, все из которых сконцентрированы в одном сложном else-блоке.

def open_decoded_file(path) do
  with {:ok, encoded} <- File.read(path),
       {:ok, decoded} <- Base.decode64(encoded) do
    {:ok, String.trim(decoded)}
  else
    {:error, _} -> {:error, :badfile}
    :error -> {:error, :badencoding}
  end
end

В приведённом коде неясно, как каждое условие слева от <- относится к соответствующей ошибке в конце. Чем больше условий в with, тем меньше ясности в коде и тем больше вероятность перекрытия не связанных между собой ошибок.

Переработка

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

def open_decoded_file(path) do
  with {:ok, encoded} <- file_read(path),
       {:ok, decoded} <- base_decode64(encoded) do
    {:ok, String.trim(decoded)}
  end
end

defp file_read(path) do
  case File.read(path) do
    {:ok, contents} -> {:ok, contents}
    {:error, _} -> {:error, :badfile}
  end
end

defp base_decode64(contents) do
  case Base.decode64(contents) do
    {:ok, decoded} -> {:ok, decoded}
    :error -> {:error, :badencoding}
  end
end

Сложные извлечения в разделах

Проблема

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

Пример

Функция drive/1 с несколькими разделами извлекает поля структуры %User{} для использования в выражении раздела (age) и для использования в теле функции (%%%CODE_BLOCK_22%%):

def drive(%User{name: name, age: age}) when age >= 18 do
  "#{name} can drive"
end

def drive(%User{name: name, age: age}) when age < 18 do
  "#{name} cannot drive"
end

Хотя приведенный пример небольшой и не демонстрирует антипаттерн, он является примером смешанного извлечения и сопоставления с образцом. В ситуациях, когда drive/1 было бы более сложным, имея больше разделов, аргументов и извлечений, было бы сложно понять на первый взгляд, какие переменные используются для сопоставления с образцом/условия, а какие нет.

Переработка

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

def drive(%User{age: age} = user) when age >= 18 do
  %User{name: name} = user
  "#{name} can drive"
end

def drive(%User{age: age} = user) when age < 18 do
  %User{name: name} = user
  "#{name} cannot drive"
end

Динамическое создание атомов

Проблема

Атом — это базовый тип в Elixir, чьё значение — это собственное имя. Атомы часто полезны для идентификации ресурсов или выражения состояния или результата операции. Создание атомов динамически само по себе не является антипаттерном; однако атомы не собираются сборщиком мусора виртуальной машины Erlang, поэтому значения этого типа хранятся в памяти в течение всего жизненного цикла программного обеспечения. По умолчанию виртуальная машина Erlang ограничивает количество атомов, которые могут существовать в приложении, значением 1 048 576, что более чем достаточно для всех атомов, определённых в программе, но попытки ограничить приложения, которые «вытесняют атомы» посредством динамического создания, являются целесообразными.

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

Пример

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

defmodule MyRequestHandler do
  def parse(%{"status" => status, "message" => message} = _payload) do
    %{status: String.to_atom(status), message: message}
  end
end

iex> MyRequestHandler.parse(%{"status" => "ok", "message" => "all good"})
%{status: :ok, message: "all good"}

При использовании функции String.to_atom/1 для динамического создания атома, она фактически получает потенциальный доступ к созданию произвольных атомов в нашей системе, что приводит к потере контроля над соблюдением ограничений, установленных BEAM. Эта проблема может быть использована для создания достаточного количества атомов для остановки системы.

Переработка

Для устранения этого антипаттерна разработчики должны либо выполнять явные преобразования, сопоставляя строки с атомами, либо заменить использование String.to_atom/1 на String.to_existing_atom/1. Явное преобразование может быть выполнено следующим образом:

defmodule MyRequestHandler do
  def parse(%{"status" => status, "message" => message} = _payload) do
    %{status: convert_status(status), message: message}
  end

  defp convert_status("ok"), do: :ok
  defp convert_status("error"), do: :error
  defp convert_status("redirect"), do: :redirect
end

iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" => "all good"})
** (FunctionClauseError) no function clause matching in MyRequestHandler.convert_status/1

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

Альтернативой является использование String.to_existing_atom/1, которое преобразует строку в атом только в том случае, если атом уже существует в системе:

defmodule MyRequestHandler do
  def parse(%{"status" => status, "message" => message} = _payload) do
    %{status: String.to_existing_atom(status), message: message}
  end
end

iex> MyRequestHandler.parse(%{"status" => "status_not_seen_anywhere", "message" => "all good"})
** (ArgumentError) errors were found at the given arguments:

  * 1st argument: not an already existing atom

В таких случаях передача неизвестного состояния вызовет ошибку, если состояние не определено как атом нигде в системе. Однако, предполагая, что status может быть либо :ok, либо :error, либо :redirect, как можно гарантировать, что эти атомы существуют? Вы должны гарантировать, что эти атомы существуют где-то в том же модуле, где вызывается String.to_existing_atom/1. Например, если у вас есть такой код:

defmodule MyRequestHandler do
  def parse(%{"status" => status, "message" => message} = _payload) do
    %{status: String.to_existing_atom(status), message: message}
  end

  def handle(%{status: status}) do
    case status do
      :ok -> ...
      :error -> ...
      :redirect -> ...
    end
  end
end

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

def valid_statuses do
  [:ok, :error, :redirect]
end

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

Длинный список параметров

Проблема

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

Пример

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

defmodule Library do
  # Too many parameters that can be grouped!
  def loan(user_name, email, password, user_alias, book_title, book_ed) do
    ...
  end
end

Переработка

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

Для этого конкретного примера аргументы для loan/6 можно сгруппировать в две разные карты, тем самым уменьшив её арность до loan/2.

defmodule Library do
  def loan(%{name: name, email: email, password: password, alias: alias} = user, %{title: title, ed: ed} = book) do
    ...
  end
end

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

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

Нарушение пространства имён

Проблема

Этот антипаттерн проявляется, когда автор пакета или библиотеки определяет модули за пределами своего «пространства имён». Библиотека должна использовать своё имя в качестве «префикса» для всех своих модулей. Например, пакет с именем :my_lib должен определять все свои модули в пространстве имён MyLib, например, MyLib.User, MyLib.SubModule, MyLib.Application, и сам MyLib.

Это важно, потому что виртуальная машина Erlang может загрузить только один экземпляр модуля за раз. Таким образом, если несколько библиотек определяют один и тот же модуль, то они несовместимы друг с другом из-за этого ограничения. Используя имя библиотеки в качестве префикса, вы избегаете конфликтов имён модулей из-за уникального префикса.

Пример

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

defmodule Plug.Auth do
  # ...
end

Даже если Plug сейчас не определяет модуль Plug.Auth, она может добавить такой модуль в будущем, что в конечном итоге приведёт к конфликту с определением plug_auth.

Переработка

Учитывая, что пакет называется :plug_auth, он должен определять модули внутри пространства имён PlugAuth:

defmodule PlugAuth do
  # ...
end

Дополнительные замечания

Существует несколько известных исключений из этого антипаттерна:

• Реализации протоколов по своему дизайну определяются в пространстве имён протокола.

• В некоторых сценариях владелец пространства имён может разрешить исключения из этого правила. Например, в самом Elixir вы определяете собственные задачи Mix, помещая их в пространство имён Mix.Tasks, например, Mix.Tasks.PlugAuth.

• Если вы являетесь хранителем и plug, и plug_auth, то вы можете разрешить plug_auth определять модули с пространством имён Plug, например, Plug.Auth. Однако вы несёте ответственность за предотвращение или управление потенциальными конфликтами в будущем.

Доступ к карте без утверждений

Проблема

В Elixir можно обращаться к значениям из Map, которые представляют собой структуры данных "ключ-значение", статически или динамически.

Когда ожидается, что ключ существует в карте, необходимо использовать запись map.key, чтобы четко указать разработчикам (и компилятору), что ключ должен существовать. Если ключ не существует, генерируется исключение (а в некоторых случаях также предупреждения компилятора). Это также известно как статическая запись, поскольку ключ известен на момент написания кода.

Когда ключ является необязательным, необходимо использовать запись map[:key] вместо этого. Таким образом, если указанный ключ не существует, возвращается nil. Это динамическая запись, так как она также поддерживает динамический доступ к ключам, например, map[some_var].

Если вы используете map[:key] для доступа к ключу, который всегда существует в карте, код становится менее понятным для разработчиков и компилятора, поскольку им теперь необходимо учитывать возможность отсутствия ключа. Это несоответствие также может усложнить отслеживание определённых ошибок. Если ключ неожиданно отсутствует, значение nil будет распространяться по системе вместо вызова исключения при доступе к карте.

Пример

Функция plot/1 пытается отобразить графическое представление позиции точки в декартовой системе координат. Эта функция принимает параметр типа Map с атрибутами точки, которые могут представлять точку 2D или 3D декартовой системы координат. Эта функция использует динамический доступ для извлечения значений по ключам карты:

defmodule Graphics do
  def plot(point) do
    # Some other code...
    {point[:x], point[:y], point[:z]}
  end
end

iex> point_2d = %{x: 2, y: 3}
%{x: 2, y: 3}
iex> point_3d = %{x: 5, y: 6, z: 7}
%{x: 5, y: 6, z: 7}
iex> Graphics.plot(point_2d)
{2, 3, nil}
iex> Graphics.plot(point_3d)
{5, 6, 7}

Поскольку мы хотим отображать как 2D, так и 3D точки, ожидается такое поведение. Но что произойдёт, если мы забудем передать точку с ключом :x или :y?

iex> bad_point = %{y: 3, z: 4}
%{y: 3, z: 4}
iex> Graphics.plot(bad_point)
{nil, 3, 4}

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

Переработка

Для устранения этого антипаттерна необходимо использовать синтаксис динамического map[:key] и статическую запись map.key в соответствии с нашими требованиями. Мы ожидаем, что :x и :y всегда существуют, но не :z. Следующий код демонстрирует переработку plot/1, устраняя этот антипаттерн:

defmodule Graphics do
  def plot(point) do
    # Some other code...
    {point.x, point.y, point[:z]}
  end
end

iex> Graphics.plot(point_2d)
{2, 3, nil}
iex> Graphics.plot(bad_point)
** (KeyError) key :x not found in: %{y: 3, z: 4} # <= explicitly warns that
  graphic.ex:4: Graphics.plot/1                  # <= the :x key does not exist!

В целом, использование map.key и map[:key] кодирует важную информацию о вашей структуре данных, позволяя разработчикам чётко выражать свои намерения. Подробнее см. документацию модуля Map и Access.

Альтернативой переработке этого антипаттерна является использование сопоставления с образцом, определяя явные правила для 2D и 3D точек:

defmodule Graphics do
  # 3d
  def plot(%{x: x, y: y, z: z}) do
    # Some other code...
    {x, y, z}
  end

  # 2d
  def plot(%{x: x, y: y}) do
    # Some other code...
    {x, y}
  end
end

Сопоставление с образцом особенно полезно при сопоставлении по нескольким ключам, а также по самим значениям.

Другой вариант — использование структур. По умолчанию структуры поддерживают только статический доступ к своим полям. В таких сценариях вы можете рассмотреть возможность определения структур для 2D и 3D точек:

defmodule Point2D do
  @enforce_keys [:x, :y]
  defstruct [x: nil, y: nil]
end

В целом, структуры полезны при обмене структурами данных между модулями, но сопряжены с зависимостью от времени компиляции между этими модулями. Если модуль A использует структуру, определённую в модуле B, то A необходимо перекомпилировать, если поля в структуре B изменятся.

Дополнительные замечания

Этот антипаттерн ранее был известен как Доступ к несуществующим полям карты/структуры.

Неуверенное сопоставление с образцом

Проблема

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

Пример

Функция get_value/2 пытается извлечь значение из определённого ключа строки запроса URL. Поскольку она не реализована с использованием сопоставления с образцом, get_value/2 всегда возвращает значение, независимо от формата строки запроса URL, переданного в качестве параметра вызова. Иногда возвращаемое значение будет действительным. Однако, если в вызове используется строка запроса URL с неожиданным форматом, get_value/2 извлечёт некорректные значения из неё:

defmodule Extract do
  def get_value(string, desired_key) do
    parts = String.split(string, "&")

    Enum.find_value(parts, fn pair ->
      key_value = String.split(pair, "=")
      Enum.at(key_value, 0) == desired_key && Enum.at(key_value, 1)
    end)
  end
end

# URL query string with the planned format - OK!
iex> Extract.get_value("name=Lucas&university=UFMG&lab=ASERG", "lab")
"ASERG"
iex> Extract.get_value("name=Lucas&university=UFMG&lab=ASERG", "university")
"UFMG"
# Unplanned URL query string format - Unplanned value extraction!
iex> Extract.get_value("name=Lucas&university=institution=UFMG&lab=ASERG", "university")
"institution"   # <= why not "institution=UFMG"? or only "UFMG"?

Переработка

Чтобы устранить этот антипаттерн, get_value/2 может быть переработана с использованием сопоставления с образцом. Таким образом, если используется неожиданный формат строки запроса URL, функция завершится с ошибкой вместо того, чтобы возвращать неверное значение. Это поведение, показанное ниже, позволяет клиентам решать, как обработать эти ошибки, и не создаёт ложного впечатления о корректной работе кода при извлечении неожиданных значений:

defmodule Extract do
  def get_value(string, desired_key) do
    parts = String.split(string, "&")

    Enum.find_value(parts, fn pair ->
      [key, value] = String.split(pair, "=") # <= pattern matching
      key == desired_key && value
    end)
  end
end

# URL query string with the planned format - OK!
iex> Extract.get_value("name=Lucas&university=UFMG&lab=ASERG", "name")
"Lucas"
# Unplanned URL query string format - Crash explaining the problem to the client!
iex> Extract.get_value("name=Lucas&university=institution=UFMG&lab=ASERG", "university")
** (MatchError) no match of right hand side value: ["university", "institution", "UFMG"]
  extract.ex:7: anonymous fn/2 in Extract.get_value/2 # <= left hand: [key, value] pair
iex> Extract.get_value("name=Lucas&university&lab=ASERG", "university")
** (MatchError) no match of right hand side value: ["university"]
  extract.ex:7: anonymous fn/2 in Extract.get_value/2 # <= left hand: [key, value] pair

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

case/2 — ещё один важный конструкт в Elixir, который помогает нам писать утвердительный код, сопоставляя с конкретными шаблонами. Например, если функция возвращает {:ok, ...} или {:error, ...}, предпочтительнее явно сопоставлять с обоими шаблонами:

case some_function(arg) do
  {:ok, value} -> # ...
  {:error, _} -> # ...
end

В частности, избегайте сопоставления только с _, как показано ниже:

case some_function(arg) do
  {:ok, value} -> # ...
  _ -> # ...
end

Сопоставление с _ менее понятно по намерениям и может скрывать ошибки, если some_function/1 добавит новые возвращаемые значения в будущем.

Дополнительные замечания

Этот антипаттерн ранее был известен как Спекулятивные предположения.

Неутвердительное истинностное значение

Проблема

Elixir предоставляет понятие истинностного значения: nil и false считаются «ложными», а все остальные значения — «истинными». Многие конструкции языка, такие как &&/2, ||/2 и !/1, обрабатывают истинные и ложные значения. Использование этих операторов не является антипаттерном. Однако использование этих операторов, когда все операнды ожидаются как булевы значения, может быть антипаттерном.

Пример

Простейший сценарий проявления этого антипаттерна — в условных операторах, таких как:

if is_binary(name) && is_integer(age) do
  # ...
else
  # ...
end

Поскольку оба операнда оператора &&/2 — булевы значения, код более общий, чем требуется, и потенциально менее понятен.

Переработка

Для устранения этого антипаттерна можно заменить &&/2, ||/2 и !/1 соответственно на and/2, or/2 и not/1. Эти операторы предполагают, что хотя бы первый аргумент — это булево значение:

if is_binary(name) and is_integer(age) do
  # ...
else
  # ...
end

Этот метод может быть особенно важен при работе с кодом Erlang. Erlang не имеет понятия истинностного значения. Он никогда не возвращает nil, вместо этого его функции могут возвращать :error или :undefined в местах, где разработчик Elixir вернул бы nil. Поэтому, чтобы случайно не интерпретировать :undefined или :error как истинное значение, лучше использовать and/2, or/2 и not/1 исключительно при взаимодействии с API Erlang.