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

В данном документе описаны потенциальные антипаттерны, связанные с кодом, конкретными идиомами и особенностями 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) и для использования в теле функции (name):

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

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

Проблема

Атом Atom — это базовый тип 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}
  graphic.ex:4: Graphics.plot/1

В целом, использование 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.