Эвристические правила и "запахи" кода

Вдохновившись книгой Роберта Мартина "Чистый код. Создание, анализ и рефакторинг", решил собрать в одном месте конспект по одной из глав, посвящённой заголовку данной статьи.

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

Быстрая навигация:

Комментарии

C1: Неуместная информация

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

Так же неуместными являются очевидные комментарии и комментарии, разделяющие логические блоки кода:

// Хедер
...

// Фильтр
...

// Футер
...

Всё это является признаками необходимости в рефакторинге, а не - в пояснении.

C2: Устаревший комментарий

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

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

C3: Избыточный комментарий

Избыточным считается комментарий, описывающий то, что и так очевидно, например:

i++; // Увеличение переменной i

Комментарии должны говорить то, что не может сказать сам код.

В лучшем случае такой код стоит переписать.

C4: Плохо написанный комментарий

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

C5: Закомментированный код

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

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

Рабочая среда

E1: Построение состоит из нескольких этапов

В данном контексте слово "построение" используется в качестве привычного "билд".

Построение должно быть одной тривиальной операцией. Без длинных серий невразумительных команд или контекстно-зависимых сценариев для построения отдельных элементов.

Одна команда на получение проекта, одна - для билда.

# build.sh

git clone git@github.com:author/project.git
cd project
npm run build

E2: Тестирование состоит из нескольких этапов

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

Функции

F1: Слишком много аргументов

Функции должны иметь небольшое количество аргументов. Лучше всего, когда аргументов вообще нет [речь о методах]; далее следуют функции с одним, двумя и тремя аргументами.

Функции с бо́льшим количество аргументов сложно читаются и воспринимаются. Их порядок аргументов всегда вызывает сомнение и заставляет обращаться к исходникам или документации. Большое количество аргументов свидетельствует о необходимости класса, либо - объединения в объект, если аргументы имеют тесную связь:

// плохо
// 1. много аргументов
// 2. аргументы и имя функции находятся на разных уровнях абстракции
new Line(x1, y1, x2, y2);

// хорошо
const point1 = new Point(x, y);
const point2 = new Point(x2, y2);

new Line(point1, point2);

// плохо - порядок аргументов не интуитивен и его невозможно запомнить
renderAuthor('firstName', lastName, age, articlesCount, keySkills);

// хорошо
const author = {
  firstName,
  lastName,
  age,
  articleCount,
  keySkills,
};

renderAuthor(author);

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

Если аргументов нет - задача тривиальна.

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

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

F3: Флаги в аргументах

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

Для таких функций сложно предугадать поведение, глядя только на её вызов.

// плохо - что такое true?
range(0, 10, true)

// хорошо
if (inclusive) {
  getRangeInclusive(0, 10)
} else {
  getRange(0, 10)
}

Явное лучше, чем неявное.

F4: Мёртвые функции

Если метод ни разу не вызывается в программе, то его следует удалить. Не бойтесь удалять мёртвые функции.

В любом случае они сохранятся в истории вашей системы контроля версий.

Разное

G1: Несколько языков в одном исходном файле

Современные среды программирования позволяют объединять в одном исходном файле код, написанный на разных языках. Например, исходный файл на языке Java может содержать вставки XML, HTML, YAML, JavaDoc, Javascript и т.д. В лучшем случае результат получается запутанны, а в худшем - неаккуратным и ненадёжным.

На практике непросто избежать смешивания языков, например, когда это продиктовано самой технологией (JSX). Но по крайней мере следует стараться свести к минимуму объём кода на дополнительных языках внутри одного исходного файла.

G2: Очевидное поведение не реализовано

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

const monthName = AnyDateLib.getMonthName(1);

console.log(monthName); // ?

Разберём очевидные моменты, которые ожидаешь от такой функции при чтении её вызова.

#1. Раз мы запрашиваем имя месяца, следовательно, вызывать функцию хотим, передавая в неё индекс месяца.

#2. Начинается ли индекс с нуля или единицы? Логично предположить по аналогии со стандартной библиотекой Date, что индексы месяца начинаются с нуля.

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

#4. С большой или маленькой буквы? Опять же, в жизни более привычно видеть начало фразы с большой буквы. К тому же в любом языке программирования есть средства, позволяющие поменять регистр той или иной буквенной последовательности, что позволит изменить возвращаемый формат.

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

Таким образом, в итоге я бы хотел ожидать от этого кода результат "Февраль". Если я получу "янв.", то буду крайне удивлён.

G3: Некорректное граничное поведение

Код должен работать правильно. Написав очередную функцию, поведение которой кажется правильным, легко упустить момент её работы с граничными условиями. Например, вы написали функцию расчёта последовательности Фибоначчи. Как она будет работать, если вызвать её с отрицательным числом? А - с очень большим числом? А если и вовсе забыть передать параметр?

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

G4: Отключённые средства безопасности

Отключать средства безопасности рискованно. Иногда отключение некоторых (или всех!) предупреждений компилятора позволяет успешно построить программу, но при этом вы рискуете бесконечными отладочными сеансами. Не отключайте сбойные тесты, обещая себе, что вы заставите их проходить позднее, - это так же неразумно, как считать кредитную карту источником бесплатных денег.

При сборке проектов в режиме разработки не используйте флаги --silent, --no-warnings и т.д. Таким образом вы лишаете себя возможности среагировать на ошибку на ранней стадии её появления.

G5: Дублирование

Одно из важнейших правил. Каждое повторение - претендент на абстракцию. Следуйте принципам DRY, SOLID и проектируйте код до его написания. Выделяйте высокоуровневые абстракции и создавайте конкретные реализации. Другие программисты могут воспользоваться вашими абстрактными концепциями, что снизит уровень ошибок и увеличит скорость разработки.

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

Преднамеренное дублирование хорошо для выявления неочевидных абстракций. Но в то же время не каждое дублирование это кандидат на новый метод. Например, цепочки if/else, switch/case, где более уместно будет применение полиморфизма:

// плохо
const people = [
  { speakLanguage: 'russian' },
  { speakLanguage: 'english' },
];

const sayHello = people => {
  people.forEach(man => {
    switch (man.speakLanguage) {
      case 'russian': {
        return 'Привет';
      }
      case 'english': {
        return 'Hello';
      }
    }
  });
}

sayHello(people);

// хорошо
class Man {
  /**
   * @abstract
   */
  sayHello() {}
}

class Russian extends Man {
  sayHello() {
    return 'Привет';
  }
}

class American extends Man {
  sayHello() {
    return 'Hello';
  }
}

const people = [
  new Russian(),
  new American(),
];

const sayHello = people => {
  people.forEach(man => {
    man.sayHello();
  });
}

sayHello(people);

Ещё сложнее модули со сходными алгоритмами, не содержащие похожих строк кода. Однако дублирование присутствует и в этом случае. Проблема решается применением паттернов ШАБЛОННЫЙ МЕТОД или СТРАТЕГИЯ.

Само объектно-ориентированное программирование может рассматриваться как стратегия модульной организации кода и устранения дубликатов. Естественно, это относится и к структурному программированию.

G6: Код на неверном уровне абстракции

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

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

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

// плохо
| components
|-- UserForm
|-- userFieldValidation.js

// хорошо
| modules
|-- userForm
  |-- components
    |-- UserForm
    |-- ...
  |-- helpers
    |-- userFieldValidation.js
    |-- ...

Рассмотрим интерфейс с неверно спроектированным уровнем абстракции:

// плохо
interface Stack {
  pop();
  push();
  percentFull();
}

Метод percentFull находится на неверном уровне абстракции. Существует ряд реализаций стэка, для которых концепция заполнения выглядит разумной, однако не все реализации могут точно сказать это. Следовательно, такую функцию следует вынести в один из производных интерфейсов:

// хорошо
interface Stack {
  pop();
  push();
}

interface BoundedStack extends Stack {
  percentFull();
}

А что, если всё-таки оставить метод percentFull в базовом интерфейсе и для его реализация по умолчанию возвращать ноль или -1? Здесь возникает проблема из реального мира, где бесконечного стэка не существует и рано или поздно вы столкнётесь с out of memory исключением. Более того, подобная реализация по умолчанию в производных классах будет явно врать.

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

G7: Базовые классы, зависящие от производных

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

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

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

G8: Слишком много информации

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

// плохо
class User {
  confirmRegistration();
  sendWelcomeEmail();
}

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

// хорошо
interface SignUpFlow {
  confirm();
  finish();
}

class EmailSignUpFlow implements SignUpFlow {}

class PhoneSignUpFlow implements SignUpFlow {}

class UserSignUp {
  constructor(signUpFlow: SignUpFlow) {
    this.signUpFlow = signUpFlow;
  }

  process() {
    this.signUpFlow.confirm();
    this.signUpFlow.finish();
  }
}

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

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

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

G9: Мёртвый код

Мёртвым кодом называется код, не выполняемый в ходе работы программы. Он содержится в теле командыif, проверяющей невозможное условие. Он содержится в секции catch для блокаtry, никогда не инициирующего исключения.

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

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

G10: Вертикальное разделение

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

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

G11: Непоследовательность

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

Если функция, работает с переменной res, которой присвоен ответ HttpResponse то используйте такое же имя переменной и в других функциях, работающих с HttpResponse. Протягивайте схожую схему именования по всему вашему коду.

// плохо
processVerificationRequest();
makeUpdateRequest();
processDeletion();

// хорошо
processVerificationRequest();
processUpdateRequest();
processDeleteRequest();

G12: Балласт

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

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

G13: Искусственные привязки

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

// плохо
class User {
  static validateEmail();
}

// хорошо
class StringValidator {
  static validateEmail();
}

Искусственная привязка возникает в результате размещения переменной, константы или функции во временно удобном, но не подходящем месте. Главные причины для появления искусственных привязок - лень и небрежность. Слишком часто мы размещаем их в удобном месте "под рукой", а потом оставляем там навсегда.

G14: Функциональная зависть

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

class Hourly PayCalculator {
  calculateWeeklyPay(e: HourlyEmployee): Money {
    const tenthRate = e.getTenthRate().getPennies();
    const tenthsWorked = e.getTenthWorked();
    const straightTime = Math.min(400, tenthsWorked);
    const overTime = Math.max(0, tenthsWorked - straightTime);
    const straightPay = straightTime * tenthRate;
    const overtimePay = Math.round(overTime * tenthRate * 1.5);

    return new Money(straightPay + overtimePay);
  }
}

Метод calculateWeeklyPay обращается к объекту HourlyEmployee за данными для обработки. Метод calculateWeeklyPay завидует области видимости HourlyEmployee. Он "желает" получить доступ к внутренней реализации HourlyEmployee.

G15: Аргументы-селекторы

Ничто так не раздражает, как висящий в конце вызова функции аргумент false. Что изменится, если этот аргумент будет равен true? Смысл селектора трудно запомнить, но дело не только в этом - селектор указывает на объединение нескольких функций в одну. В общем случае лучше иметь несколько функций, чем передавать функции признак для выбора поведения.

// плохо
function render(isServer) {}

// хорошо
function renderForClient() {}

function renderForServer() {}

G16: Непонятные намерения

Код должен быть как можно более выразительным. Слишком длинные выражения, венгерская запись, "волшебные числа" - всё это скрывает намерения автора.

G17: Неверное размещение

Одно из самых важных решений, принимаемых разработчиком, - выбор места для размещения кода. Например, где следует объявить константу PI? В классе Math? А может, ей место в классе Trigonometry? Или в классе Circle?

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

G18: Неуместные статические методы

Math.max(a, b) - хороший статический метод. Он работает не с одним экземпляром; в самом деле запись вида new Math().max(a, b) или даже a.max(b) выглядела бы довольно глупо. Все данные используемые max, берутся из двух аргументов, а не из некоего объекта-"владельца". А главное, что метод Math.max почти наверняка не потребуется делать полиморфным.

Иногда программисты пишут функции, которые статическими быть не должны. Например:

HourlyPayCalculator.calculatePay(employee, overtimeRate);

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

// плохо
HourlyPayCalculator.calculatePay(employee, overtimeRate);
OvertimeHourlyPayCalculator.calculatePay(employee, overtimeRate);

// хорошо
interface PayCalculator {
  calculate();
}

class HourlyPayCalculator implements PayCalculator {}
class OvertimeHourlyPayCalculator implements PayCalculator {}

class Employee {
  calculatePay(payCalculator: PayCalculator) {
    return payCalculator.calculate();
  }
}

G19: Используйте пояснительные переменные

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

// плохо
const match = rawHeaderLine.match(headerPattern);

headers.set(match[1].toLowerCase(), match[2]);

// хорошо
const match = rawHeaderLine.match(headerPattern);
const key = match[1];
const value = match[2];

headers.set(key.toLowerCase(), value);

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

G20: Имена функций должны описывать выполняемую операцию

Если вам приходится обращаться к реализации (или документации), чтобы понять, что делает та или иная функция, постарайтесь найти более удачное имя или разбейте функциональность на меньшие функции с более понятными именами.

// плохо
const newDate = date.add(5);

Как вы думаете, что делает этот код - прибавляет пять дней к date? А может, пять недель или часов? Изменяется ли экземпляр date, или функция возвращает новое значение Date без изменения старого? По вызову невозможно понять, что делает эта функция.

// хорошо
date.addDaysTo(5);

// хорошо
const newDate = date.daysLater(5);

G21: Понимание алгоритма

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

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

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

G22: Преобразование логических зависимостей в физические

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

// плохо
class FilmController {
  openDetails() {
    const popup = new Popup();
    this.popupOpened = true;
  }

  isPopupOpened() {
    return this.popupOpened;
  }
}

// хорошо
class FilmController {
  openDetails() {
    this.popup = new Popup();
  }

  isPopupOpened() {
    return this.popup.isOpened();
  }
}

G23: Используйте полиморфизм вместо if/else или switch/case

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

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

G24: Соблюдайте стандартные конвенции

Все рабочие группы должны соблюдать единые стандарты кодирования, основанные на отраслевых нормах. Стандарт кодирования определяет, где объявляются переменные экземпляров; как присваиваются имена классов, методов и переменных; где размещаются фигурные скобки и т.д. Документ с явным описанием этих правил не нужен - сам код служит примером оформления.

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

G25: Заменяйте "волшебные числа" именованными константами

Термин "волшебное число" относиться не только к числам. Он распространяется на все лексемы, значения которых не являются самодокументирующими. Некоторые числа так легко узнаются, что их не обязательно скрывать за именованными константами - при условии, что они используются в сочетании с предельно ясным кодом.

// плохо
setTimeout(() => {}, 86400);

// хорошо
const SECONDS_PER_DAY = 86400;

setTimeout(() => {}, SECONDS_PER_DAY);

// плохо
const TWO = 2;
const circumference = radius * Math.PI * TWO;

// хорошо
const circumference = radius * Math.PI * 2;

G26: Будьте точны

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

Неоднозначности и неточности в коде объясняются либо недопониманием, либо ленью. В любом случае от них следует избавиться.

G27: Структура важнее конвенций

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

G28: Инкапсулируйте условные конструкции

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

// плохо
if (time.hasExpired() && !timer.isRecurrent())

// хорошо
if (shouldBeDeleted(timer))

G29: Избегайте отрицательных условий

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

// плохо
if (!buffer.shouldNotCompact())

// хорошо
if (buffer.shouldCompact())

G30: Функции должны выполнять одну операцию

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

// плохо
function pay() {
  for (e of employees) {
    if (e.isPayday()) {
      const pay = e.calculatePay();

      e.deliverPay(pay);
    }
  }
}

// хорошо
function pay() {
  for (e of employees) {
    payIfNecessary(e);
  }
}

function payIfNecessary(employee) {
  if (employee.isPayday()) {
    calculateAndDeliverPay(employee);
  }
}

function calculateAndDeliverPay(employee) {
  const pay = employee.calculatePay();

  employee.deliverPay(pay);
}

G31: Скрытые временны́е привязки

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

// плохо
class MoogDiver {
  gradient;
  splines;

  dive(reason) {
    saturateGradient();
    reticulateSplines();
    diveForMoog();
  }
}

// хорошо
class MoogDiver {
  gradient;
  splines;

  dive(reason) {
    const gradient = saturateGradient();
    const splines = reticulateSplines(gradient);

    diveForMoog(splines, reason);
  }
}

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

G32: Структура кода должна быть обоснована

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

G33: Инкапсулируйте граничные условия

Отслеживать граничные условия нелегко. Разместите их обработку в одном месте. Не позволяйте им "растекаться" по всему коду. Не допускайте, чтобы в вашей программе кишели многочисленные +1 и -1.

// плохо
if (level + 1 < tags.length) {
  const parts = new Parse(body, tags, level + 1, offset + endTag);
}

// хорошо
const nextLevel = level + 1;

if (nextLevel < tags.length) {
  const parts = new Parse(body, tags, nextLevel, offset + endTag);
}

G34: Функции должны быть написаны на одном уровне абстракции

Все команды функции должны быть сформулированы на одном уровне абстракции, который расположен одним уровнем ниже операции, описываемой именем функции.

// плохо
render() {
  const html = '<hr';

  if (this.extraDashes > 0) {
    html += ` size="${this.extraDashes + 1}"`;
  }

  return `${html} />`;
}

В этом методе смешиваются минимум два уровня абстракции. Первый уровень - наличие толщины у горизонтальной линии. Второй уровень - синтаксис тега HR.

// хорошо
render() {
  const hr = new HtmlTag('hr');

  if (this.extraDashes > 0) {
    hr.addAttribute('size', this.hrSize(this.extraDashes));
  }

  return hr.toHtml();
}

hrSize(height) {
  return String(height + 1);
}

Изменение разделяет два уровня абстракции. Функция render просто конструирует тег HR, ничего не зная о синтаксисе HTML этого тега. Модуль HtmlTag берёт на себя все хлопоты с синтаксисом.

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

G35: Храните конфигурационные данные на высоких уровнях

Если в программе имеется константа, определяющая значение по умолчанию или параметр конфигурации, и эта константа известна на высоких уровнях абстракции, - не прячьте её в низкоуровневой функции. Передайте её в аргументе низкоуровневой функции, вызываемой из функции высокого уровня.

G36: Избегайте транзитивных обращений

В общем случае модуль не должен обладать слишком полной информацией о тех компонентах, с которыми он взаимодействует. Точнее, если A взаимодействует с B, а B взаимодействует с C, то модули, использующие A, не должны знать о C (то есть нежелательны конструкции вида a.getB().getC().doSomething()). Иногда это называется "законом Деметры".

Основная проблема знания о внутренних компонентах в отсутствии гибкости в расширяемости и слишком тесная связность кода. Допустим, вы использовали знания о модуле C из модуля A в нескольких местах. В дальнейшем вам будет трудно изменить архитектуру, когда между этими модулями появится новый модуль Q, т.к. придётся искать все подобные вхождения и добавлять туда промежуточный модуль. Цепочка подобных вызовов явно говорит о связности и деталях реализации. Проблема должна решаться простыми вызовами вида:

myModule.doSomethong();

Имена

N1: Используйте содержательные имена

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

Не рассматривайте это как дополнительный "фактор комфортности". Сила хорошо выбранных имён заключается в том, что они дополняют структуру кода описаниями. Имена в программном продукте на 90% определяют удобочитаемость кода. Имена слишком важны, чтобы относиться к ним легкомысленно.

N2: Выбирайте имена на подходящем уровне абстракции

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

// плохо
addEventListener('click', onPressCtrlEnter);

// хорошо
addEventListener('click', onSendMessage);

N3: По возможности используйте стандартную номенклатуру

Имена проще понять, если они основаны на существующих конвенциях или стандартных обозначениях. Например, при использовании паттерна ДЕКОРАТОР можно включить в имена декорирующих классов слово Decorator.

// плохо
function increment(amount) {
  return amount++;
}

// хорошо
function increment(count) {
  return count++;
}

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

N4: Недвусмысленные имена

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

// плохо
doRename() {
  if (this.refactorReferences) {
    this.renameReferences();
  }

  this.renamePage();
}

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

// хорошо
renamePageAndOptionallyReferences() {
  if (this.refactorReferences) {
    this.renameReferences();
  }

  this.renamePage();
}

N5: Используйте длинные имена для длинных областей видимости

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

N6: Избегайте кодирования

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

// плохо
const itemsArr = [];
const commentStr = '';

interface IArticle {}

// хорошо
const items = [];
const comment = '';

interface Article {}

N7: Имена должны описывать побочные эффекты

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

// плохо
updateComment(comment) {
  if (this.getComment(comment.id)) {
    return this.updateComment(comment.id, comment);
  }

  return this.createComment(comment);
}

// хорошо
createOrUpdateComment(comment) {
  if (this.getComment(comment.id)) {
    return this.updateComment(comment.id, comment);
  }

  return this.createComment(comment);
}

Тесты

T1: Нехватка тестов

Тестовый пакет должен тестировать всё, что может сломаться. Если в системе остались условия, не проверенные тестами, или вычисления, правильность которых не подтверждена, значит, количество тестов недостаточно.

T2: Используйте средства анализа покрытия кода

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

T3: Не пропускайте тривиальные тесты

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

T4: Отключённый тест как вопрос

Иногда мы не уверены в подробностях поведения системы, потому что неясны сами требования к программе. Вопрос о требованиях можно выразить в виде теста - закомментированного или помеченного аннотацией @Ignore. Выбор зависит от того, компилируется или нет код, к которому относится неопределённость.

T5: Тестируйте граничные условия

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

T6: Тщательно тестируйте код рядом с ошибками

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

T7: Закономерности сбоев часто несут полезную информацию

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

T8: Закономерности покрытия кода часто несут полезную информацию

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

T9: Тесты должны работать быстро

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

 Обсудить