Роберт Мартин - Чистый код. Создание, анализ и рефакторинг

Модуль в листинге 4.7 был написан для первого учебного курса «XP Immersion». Предполагалось, что он является примером плохого кодирования и стиля комментирования. Кент Бек переработал этот код в куда более приятную форму перед несколькими десятками увлеченных слушателей. Позднее я приспособил этот пример для своей книги «Agile Software Development, Principles, Patterns, and Practices» и статьи в журнале «Software Development». Любопытно, что в то время многие из нас считали этот модуль «хорошо документированным». Теперь мы видим, что он представляет собой ералаш. Посмотрим, сколько разных ошибок комментирования вам удастся найти.

/**

* Класс генерирует простые числа в диапазоне до максимального значения,

* заданного пользователем, по алгоритму "Решета Эратосфена".

*

* Эратосфен Киренский, 276 год до н.э., Ливия -- * 194 год до н.э., Александрия.

* Первый ученый, вычисливший длину земного меридиана. Известен своими работами

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

*

* Алгоритм весьма прост. Берем массив целых чисел, начиная с 2, и вычеркиваем

* из него все числа, кратные 2. Находим следующее невычеркнутое число

* и вычеркиваем все его кратные. Повторяем до тех пор, пока не дойдем

* до квадратного корня верхней границы диапазона.

*

* @author Альфонс

* @version 13 февраля 2002 u

*/

import java.util.*;

public class GeneratePrimes

{

/**

* @param maxValue - верхняя граница диапазона.

*/

public static int[] generatePrimes(int maxValue)

{

if (maxValue >= 2) // Единственно допустимый случай

{

// Объявления

int s = maxValue + 1; // Размер массива

boolean[] f = new boolean[s];

int i;

// Инициализировать массив значениями true.

for (i = 0; i < s; i++)

f[i] = true;

// Удалить числа, заведомо не являющиеся простыми.

f[0] = f[1] = false;

// Отсев

int j;

for (i = 2; i < Math.sqrt(s) + 1; i++)

{

if (f[i]) // Если элемент i не вычеркнут, вычеркнуть кратные ему.

{

for (j = 2 * i; j < s; j += i)

f[j] = false; // Кратные числа не являются простыми.

}

}

// Сколько простых чисел осталось?

int count = 0;

for (i = 0; i < s; i++)

{

if (f[i])

count++; // Приращение счетчика

}

int[] primes = new int[count];

// Переместить простые числа в результат

for (i = 0, j = 0; i < s; i++)

{

if (f[i]) // Если простое

primes[j++] = i;

}

return primes; // Вернуть простые числа

}

else // maxValue < 2

return new int[0]; // Вернуть пустой массив при недопустимых входных данных.

}

}

В листинге 4.8 приведена переработанная версия того же модуля. Обратите внимание: применение комментариев стало намного более ограниченным. Во всем модуле осталось всего два комментария пояснительного характера.

/**

* Класс генерирует простые числа до максимального значения, заданного

* пользователем, по алгоритму "Решета Эратосфена".

* Берем массив целых чисел, начиная с 2, и вычеркиваем

* из него все числа, кратные 2. Находим следующее невычеркнутое число

* и вычеркиваем все числа, кратные ему. Повторяем до тех пор, пока из массива

* не будут вычеркнуты все кратные.

*/

public class PrimeGenerator

{

private static boolean[] crossedOut;

private static int[] result;

public static int[] generatePrimes(int maxValue)

{

if (maxValue < 2)

return new int[0];

else

{

uncrossIntegersUpTo(maxValue);

crossOutMultiples();

putUncrossedIntegersIntoResult();

return result;

}

}

private static void uncrossIntegersUpTo(int maxValue)

{

crossedOut = new boolean[maxValue + 1];

for (int i = 2; i < crossedOut.length; i++)

crossedOut[i] = false;

}

private static void crossOutMultiples()

{

int limit = determineIterationLimit();

for (int i = 2; i <= limit; i++)

if (notCrossed(i))

crossOutMultiplesOf(i);

}

private static int determineIterationLimit()

{

// Каждое кратное в массиве имеет простой множитель, больший либо равный

// квадратному корню из размера массива. Следовательно, вычеркивать элементы,

// кратные числам, превышающих квадратный корень, не нужно.

double iterationLimit = Math.sqrt(crossedOut.length);

return (int) iterationLimit;

}

private static void crossOutMultiplesOf(int i)

{

for (int multiple = 2*i;

multiple < crossedOut.length;

multiple += i)

crossedOut[multiple] = true;

}

private static boolean notCrossed(int i)

{

return crossedOut[i] == false;

}

private static void putUncrossedIntegersIntoResult()

{

result = new int[numberOfUncrossedIntegers()];

for (int j = 0, i = 2; i < crossedOut.length; i++)

if (notCrossed(i))

result[j++] = i;

}

private static int numberOfUncrossedIntegers()

{

int count = 0;

for (int i = 2; i < crossedOut.length; i++)

if (notCrossed(i))

count++;

return count;

}

}

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

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

[KP78]: Kernighan and Plaugher, The Elements of Programming Style, 2d. ed., McGraw-Hill, 1978.

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