Чистый код. Создание, анализ и рефакторинг - [23]

  private Cryptographer cryptographer;

  public boolean checkPassword(String userName, String password)
{
    User user = UserGateway.findByName(userName);
    if (user != User.NULL) {
      String codedPhrase = user.getPhraseEncodedByPassword();
      String phrase = cryptographer.decrypt(codedPhrase, password);
      if ("Valid Password".equals(phrase)) {
        Session.initialize();
        return true;
      }
    }
    return false;
  }
}

Разумеется, побочным эффектом является вызов Session.initialize(). Имя checkPassword сообщает, что функция проверяет пароль. Оно ничего не говорит о том, что функция инициализирует сеанс. Таким образом, тот, кто поверит имени функции, рискует потерять текущие сеансовые данные, когда он решит проверить данные пользователя.

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

Выходные аргументы

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

appendFooter(s);

Присоединяет ли эта функция s в качестве завершающего блока к чему-то другому?  Или она присоединяет какой-то завершающий блок к s? Является ли s входным или выходным аргументом? Конечно, можно посмотреть на сигнатуру функции и получить ответ:

public void appendFooter(StringBuffer report)

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

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

report.appendFooter();

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

Разделение команд и запросов

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

public boolean set(String attribute, String value);

Функция присваивает значение атрибуту с указанным именем и возвращает true, если присваивание прошло успешно, или false, если такой атрибут не существует. Это приводит к появлению странных конструкций вида

if (set("username", "unclebob"))...

Представьте происходящее с точки зрения читателя кода. Что проверяет это условие? Что атрибут "username" содержит ранее присвоенное значение "unclebob"? Или что проверяет атрибуту "username" успешно присвоено значение "unclebob"? Смысл невозможно вывести из самого вызова, потому что мы не знаем, чем в данном случае является слово set — глаголом или прилагательным.

Автор предполагал, что set является глаголом, но в контексте команды if это имя скорее воспринимается как прилагательное. Таким образом, команда читается в виде «Если атрибуту username ранее было присвоено значение unclebob», а не «присвоить атрибуту username значение unclebob, и если все прошло успешно, то…» Можно было бы попытаться решить проблему, переименовав функцию set в setAndCheckIfExists, но это не особенно улучшает удобочитаемость команды if. Полноценное решение заключается в отделении команды от запроса, чтобы в принципе исключить любую неоднозначность.

if (attributeExists("username")) {
  setAttribute("username", "unclebob");
  ...
}

Используйте исключения вместо возвращения кодов ошибок

Возвращение кодов ошибок функциями-командами является неочевидным нарушением принципа разделения команд и запросов. Оно поощряет использование команд в предикатных выражениях if:

if (deletePage(page) == E_OK)

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

if (deletePage(page) == E_OK) {
  if (registry.deleteReference(page.name) == E_OK) {
    if (configKeys.deleteKey(page.name.makeKey()) == E_OK){
      logger.log("page deleted");
    } else {
      logger.log("configKey not deleted");
    }
  } else
{
    logger.log("deleteReference from registry failed");