Project

General

Profile

Комментарии к книге Д. Боуэлла и Т. Фаучера «Читаемый код»

Глава 2. Помещаем в имена полезную информацию

tmp

Отмечу, что использование tmp оправдано наверное лишь в 0.1% случаев. Во всяком случае, я не смог придумать больше ни одного примера (помимо приведённого в книге), где не было бы лучшего варианта.

Итераторы циклов

Очень спорное решение по поводу применения переменных с именами типа ci и mi. Если i, j и т. д. — это стандартное соглашение, то к применяемым авторами именам оно уже не относится. На мой взгляд, в приведённом случае лучше не сокращать имена переменных-индексов: club_index, member_index (несмотря на то, что они длинные).

Кроме того, во всех современных языках есть циклы foreach, и их надо использовать вместо индексов всегда, когда есть такая возможность.

Более короткие имена подходят для небольших областей видимости

Правило верное, но я бы всё же поостерёгся употреблять однобуквенные имена в принципе. Проблема коротких (и особенно однобуквенных) имён в том, что приходится смотреть на соседние строки для того, чтобы понять контекст. Код приведённого примера только выиграет от употребления для переменной имени map вместо m. Кстати, а что за имя LookupNamesNumbers()? Это вообще о чём?

Использование форматирования имён для передачи их смысла.

Несмотря на достаточную распространённость, я являюсь противником подхода, использующего префиксы (m_) и суффиксы (_) для полей класса. Главная причина — засорение кода этими префиксами и особенно суффиксами. Кроме того, среды разработки используют различные цвета для полей и локальных переменных, что позволяет с лёгкостью их различать.

Глава 3. Имена, которые нельзя понять неправильно

Пример: Filter() (фильтрация)

Предостережение по поводу select/exclude: из имени select следует, что этот метод не имеет побочных эффектов (только читает данные), тогда как про exclude этого сказать уже нельзя. Посмотрев на код

results = Database.all_objects.exclude("year <= 2011");

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

Используйте имена begin и end для включающе-исключающих границ

Из текста может сложиться впечатление, что cоглашение про begin/end является чуть ли не стандартным соглашением, действующим во всех случаях. На самом деле, это совсем не так, и end вполне может обозначать включающую границу. Более того, в английском языке (не в коде) end почти никогда не употребляется как исключающая граница.

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

Пример get*() (получать

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

Проблема здесь скорее в другом. Само слово get вызывает ощущение какого-то простого действия (просто «взять»), поэтому использование compute() в указанном примере может быть более оправданным, так как оно лучше отражает действие, выполняемое методом. Однако это же имя раскрывает некоторые детали реализации (а именно сам факт выполнения вычислений), которые остаются скрытыми, если мы используем get. Кстати, инкапсуляция также в некоторых случаях позволяет решить проблему производительности без раскрытия деталей реализации. Если мы используем внутри метода get() кэширование результатов вычислений, тогда снаружи никто может вообще не заметить разницы.

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

Пример list::size() — размер списка

Ровно то же самое, что и в предыдущем пункте. Непонятно, почему авторы считают, что метод size() должен возвращать значение немедленно — в имени метода такой информации нет — не называется же этот метод returnSizeImmediately().

Резюме по этому пункту и предыдущему: не надо перегибать палку и домысливать представления, которых на самом деле в именах нет.

Глава 4. Эстетичность

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

Перераспределение разрывов строк сделает код более последовательным и понятным

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

По поводу комментариев к значениям параметров: совершенно очевидно, что в данном случае они появились, потому что параметров слишком много. В низкоуровневом программировании этого часто не избежать, однако по-возможности следует минимизировать количество параметров. В некоторых языках есть именованные параметры (Lisp, Ruby, Python, Objective C). Такие параметры естественным образом решают обозначенную проблему.

Разбиваем код на абзацы

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

Глава 5. Комментируем мудро

Что НЕ нужно комментировать

Комментарий, приведённый в конце раздела, действительно есть смысл использовать. Несмотря на то, что в целом код достаточно идиоматичен (в Python такого рода конструкции встречаются достаточно часто), комментарий делает его несколько понятнее. Два небольших нюанса, которые следует иметь в виду при использовании подобных комментариев:

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

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

К сожалению, в корпоративной практике такого рода комментариев избежать невозможно, поскольку не существует чёткого и объективного критерия, позволяющего определить, когда комментарий не нужен. В результате появляются правила, заставляющие комментировать всё, даже getter'ы. По-другому, видимо, нельзя, потому что в противном случае результатом неизбежно будет код без комментариев вообще и программисты, убеждающие начальника отдела, что «там и так всё понятно».

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

Комментируем недостатки кода

{TODO} Затрудняюсь высказать своё мнение по обсуждаемому вопросу. Главная проблема здесь — почему описание недостатков кода помещается в код, а не в трекер? Более точно: описание каких недостатков следует помещать в код, а каких — в трекер?

Отмечаем возможные ловушки

Комментарий вида

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

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

// Время работы равно O(...)

Итоговые комментарии

Вряд ли хорошая идея делать так, как предлагается в данном разделе.

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

Глава 6. Комментарии должны быть чёткими и понятными

Описывайте цели вашего кода

Redundancy check в тексте переведено неправильно. На самом деле, это «контроль по избыточности». Имеется в виду ситуация, когда одно и то же описано двумя разными способами и/или в двух разных местах. Тогда несогласованность этих описаний свидетельствует об ошибке.

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

Комментарии, содержащие названия параметров функций

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

Глава 7. Как сделать поток команд управления удобочитаемым

Порядок аргументов в условных конструкциях

Как же меня удруает порядок аргументов во всевозможных assert'ах (в частности, в JUnit, да и в C# то же самое)! Там описанное в этом разделе правило нарушено и порядок аргументов оказывается обратным:

public static void assertEquals(long expected, long actual);

Избегайте циклов do/while

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

И ещё пара комментариев по тексту:

  • «Выражение, заключённое в этом блоке кода, может быть выполнено или не выполнено в зависимости от условия, которое располагается под ним». Автор механически переносит ситуацию цикла с предусловием на цикл do/while. Условие в таком цикле для того и расположено после тела, чтобы никто не предполагал, что оно должно быть истинно внутри — это условие продолжения действия, а не его выполнения. Кроме того, приведённое утверждение неверно и в случае цикла while — изменения переменных в теле цикла могут сделать его неверным.
  • Рассуждение про continue опять апеллирует к тому, что программист может не знать как следует языка, на котором программирует. По-моему, это недопустимо.

Кстати, код, приведённый в данном разделе, вообще очень далёк от идеала:

  • в нём изменяются входные параметры;
  • значение, которое хранится в переменной max_length соответствует своему названию только до выполнения цикла;
  • используется декремент внутри условия цикла (более того, внутри сложного условия!);
  • два приведённых варианта кода по разному обрабатывают ситуацию, когда node == null (первый вариант в этом случае завершается аварийно).
public Boolean ListHasNode(Node start_node, String name, int max_length) {
    Node node = start_node;
    int length = 0;
    while (node != null && length < max_length) {
        if (node.name.equals(name)) return true;
        node = node.next();
        length++;
    }
    return false;
}

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

Глава 8. Разбиваем длинные выражения

Поясняющие переменные; Итоговые переменные

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

if (request.user.owns_document(document)) ...

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

Разбиваем огромные утверждения

Непонятно, почему бы не сделать так:

if (vote_value === "Up") {
    thumbs_up.addClass(hi);
} else {
    thumbs_up.removeClass(hi);
}
if (vote_value === "Down") {
    thumbs_down.addClass(hi);
} else {
    thumbs_down.removeClass(hi);
}

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

var highlightWidget = function (widget, highlighted) {
    if (highlighted) {
        widget.addClass("highlighted");
    } else {
        widget.removeClass("highlighted");
    }
}

...

highlightWidget(thumbs_up, vote_value === "Up");
highlightWidget(thumbs_down, vote_value === "Down");

Ещё один творческий способ упрощения выражений

НЕ используйте макросы в C++! Вместо этого используйте методы классов! Уже по количеству обращений к getter'ам и setter'ам видно, что данная реализация должна быть инкапсулирована:

Stats& Stats::operator+=(const Stats& other) {
    total_memory += other.total_memory;
    free_memory += other.free_memory;
    swap_memory += other.swap_memory;
    num_processes += other.num_processes;
    return *this;
}

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

Глава 9. Переменные и читаемость

Сокращаем область видимости ваших переменных

Увы, здксь авторы говорят не о тех критериях выделения полей, которые на самом деле нужно использовать. Хотя в действительности появление «мини-глобальных переменных» часто свидетельствует о том, что программист поленился передать данные в другой метод через параметр, главный критерий выделения поля класса совсем другой: поле определяет состояние объекта, и это вовсе не связано с количеством методов, которые используют это поле. Таким образом, рассуждение относительно того, что переменную str_ нужно сделать локальной, вне контекста просто бессмысленно — вдруг она является частью состояния?

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

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

Глава 10. Выделяем побочные подзадачи

Вводный пример: findClosestLocation()

Хочется сделать предложить ещё одно улучшение к рассмотренному примеру. Заметим, что в коде функции findClosestLocation() чётко прослеживается вычисление величины arg min f(array[i]). Можно легко выделить функцию, которая будет вычислять эту величину:

var argmin = function(array, function)

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

Реализуйте указанные функции самостоятельно в качестве упражнения.

Функциональность, специфичная для проекта

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

Глава 11. Одна задача в любой момент времени

Нет комментариев.

Глава 12. Превращаем мысли в код

Чётко описываем логику

Пустые блоки в операторе if — плохая практика, поскольку такие блоки плохо читаются. Попробуйте прочитать вслух: «если ..., то ничего не делать; иначе...». Звучит неестественно.

Гораздо лучше выделить весь код, определяющий, что пользователь авторизован, в отдельную функцию:

<code class="php">
function user_authorized($document) {
    return is_admin_request() ||
           $document && $document['username'] == $_SESSION['username'];
}

// ...
// в коде страницы:
if (!user_authorized($document) {
    return not_authorized();
}
// ...
</code>

А ещё лучше вынести проверку факта авторизации пользователя за пределы функции, формирующей контекст страницы, или использовать для этого средства аспектно-ориентированного программирования (но вероятно, не в этом языке программирования!).

В заключение также отмечу неудачные имена функций is_admin_request() и not_authorized(), которые недостаточно точно описывают результат, возвращаемый соответствующими функциями.

Библиотеки нам помогут

Вызов cur_tip.hide() лучше поставить непосредственно перед вызовом next_tip.show(), чтобы чётко отделить часть метода, инициализирующую локальные переменные, от части метода, выполняющей действия.

Применяем этот метод к более объёмным задачам

В первую очередь хочется отметить, что пример выглядит очень сомнительным с точки зрения здравого смысла: зачем имитировать SQL-операцию INNER JOIN в коде на Python? Безусловно, в данном случае именно её и надо было применять.

Описание решения задачи на русском языке

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

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

Глава 13. Пишите меньше кода

Пример: использование инструментов UNIX вместо написания кода

Последняя фраза главы может ввести в заблуждение: «Особенно замечательно в этой команде то, что мы избежали написания „реального” кода и его проверки». Следует понимать, что приведённая UNIX-команда содержит самый настоящий код, который (в идеале) требует проверки! Другое дело, что этот код очень простой и легко читается, поэтому мы с большой долей уверенности можем утверждать, что он будет работать правильно (особенно если сравнить его с кодом, который решал бы аналогичную задачу на C++ или Java). Во многом такой эффект достигается за счёт того, что команды Shell и утилиты UNIX предоставляют гораздо более высокоуровневые средства, чем языки общего назначения.