5 видов комментариев, которые нужно перестать писать // и один – который стоит начать добавлять в свой код

1
1406
views

Перевод статьи «Five code comments you should stop writing // and one you should start».

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

Корреляция между качеством кода и комментариями

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

И нам не только рассказывали об этом – когда мы сдавали свои работы без комментариев, нас «наказывали» снижением баллов. А если вам все же удавалось избежать проверки человеком, отсутствие комментариев вылавливалось специально созданными для этого скриптами.

Возможно, на самом деле связь не прямая, а обратная

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

  1. Слишком часто комментарии являются лишь оправданием для плохо написанного кода. Программисты думают, что даже если написать грязный код со странными ходами, но при этом объяснить его пятью строками комментариев, такой код можно считать читаемым и хорошо написанным. Именно так они и делают вместо того чтобы стараться писать хороший код. Но я ответственно заявляю: такой код все равно остается плохим. Если вашему коллеге, чтобы понять ваш код, нужно прочитать длинную историю в комментариях, значит, вы написали этот код неправильно. Если ваш код не говорит сам за себя, лучше усовершенствовать его, а не использовать комментарии для пояснений.
  2. С течением времени комментарии устаревают и в результате все еще больше запутывают. Они истинны только в момент написания, но даже тогда не слишком эффективны. Со временем люди неизбежно вносят изменения в логику, меняют типы и все перемещают. При этом кто-то из программистов заметит, что нужно изменить комментарий, а кто-то – нет. И даже если вам каким-то образом удастся установить жесткую дисциплину относительно обновления комментариев, все равно она будет нарушена при первом же автоматическом рефакторинге. Представьте рефакторинг кода, при котором добавляется какой-нибудь параметр в функцию, используемую больше 250 раз – вы действительно захотите исправлять все эти комментарии вручную?

Каковы самые распространенные типы комментариев, которых стоит избегать?

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

  1. Действительно ли здесь нужен этот комментарий и насколько он важен?
  2. Можно ли улучшить этот код таким образом, чтобы комментарий не понадобился?
  3. Не служит ли этот комментарий лишь для прикрытия моего зада?

Чтобы помочь вам разобраться, я собрал 5 видов плохих комментариев, которые мне встречались. Подумайте дважды, прежде чем добавлять в свой код подобные примечания. Для наглядности я использовал примеры кода из популярных open source проектов. Не поймите неверно, я вовсе не считаю, что эти проекты плохо написаны. Наоборот, это мои любимые проекты. Но ничто в этом мире не совершенно и любой код можно улучшить.

1. Капитан Очевидность

Комментарии такого рода поясняют, что делает ваш код. Вероятно, вам такие уже встречались. Пример из react.js:

getHasteName() {
// We never want Haste.
return null;
}

И еще один – из vscode:

// Avoid Monkey Patches from Application Insights
bootstrap.avoidMonkeyPatchFromAppInsights();
// Enable portable support
bootstrap.configurePortable();
// Enable ASAR support
bootstrap.enableASARSupport();
// Load CLI through AMD loader
require('./bootstrap-amd').load('vs/code/node/cli');

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

Примечание: как и у многих других, у меня комментная слепота (по аналогии с баннерной слепотой – прим. перев.). Я игнорирую комментарии, и скорее всего при внесении изменений в код не замечу, что какой-то комментарий также нуждается в исправлении.

Возвращаясь к примеру – что произойдет, если удалить все комментарии в этом коде? Его действительно станет тяжелее читать?

2. Пояснение кода

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

Посмотрите на этот пример из react.js:

if (!existsSync('./scripts/rollup/results.json')) {
// This indicates the build failed previously.
// In that case, there's nothing for the Dangerfile to do.
// Exit early to avoid leaving a redundant (and potentially
confusing) PR comment.
process.exit(0);
}

Разве не чище стал бы этот код, если бы мы изменили его вот так:

if (buildFailedPreviously())
process.exit(0);

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

struct stack_control_header
{
long shgrow:32; /* Number of times stack has grown. */
long shaseg:32; /* Size of increments to stack. */
long shhwm:32; /* High water mark of stack. */
long shsize:32; /* Current size of stack (all segments). */
};

Если вы проскочите это описание, то не сможете сходу понять, что означают shgrow, shaseg и другие поля. Что, если переписать код вот так:

struct stack_control_header
{
long num_of_time_grown:32;
long size_of_inc:32;
long high_water_mark:32;
long current_size:32;
};

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

3. Длинные комментарии

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

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

Пример из vue.js:

// Async edge case #6566 requires saving the timestamp when event listeners are
// attached. However, calling performance.now() has a perf overhead especially
// if the page has thousands of event listeners. Instead, we take a timestamp
// every time the scheduler flushes and use that for all event listeners
// attached during that flush.
export let currentFlushTimestamp = 0
// Async edge case fix requires storing an event listener's attach timestamp.
let getNow: () => number = Date.now
// Determine what event timestamp the browser is using. Annoyingly, the
// timestamp can either be hi-res (relative to page load) or low-res
// (relative to UNIX epoch), so in order to compare time we have to use the
// same timestamp type when saving the flush timestamp.
if (inBrowser && getNow() > document.createEvent('Event').timeStamp) {
// if the low-res timestamp which is bigger than the event timestamp
// (which is evaluated AFTER) it means the event is using a hi-res timestamp,
// and we need to use the hi-res version for event listeners as well.
getNow = () => performance.now()
}

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

4. Названия, заголовки и прочие «украшения»

Код должен быть красивым, но это не означает, что его нужно украшать, как книгу. Временами у нас проявляется склонность создавать блоки кода и давать им названия, чтобы отличать их друг от друга. Давайте рассмотрим пример из angular.js:

...
build: function(config, fn) {
var files = grunt.file.expand(config.src);
// grunt.file.expand might reorder the list of files
// when it is expanding globs, so we use prefix and suffix
// fields to ensure that files are at the start of end of
// the list (primarily for wrapping in an IIFE).
if (config.prefix) {
files = grunt.file.expand(config.prefix).concat(files);
}
if (config.suffix) {
files = files.concat(grunt.file.expand(config.suffix));
}
var styles = config.styles;
var processedStyles;
//concat
var src = files.map(function(filepath) {
return grunt.file.read(filepath);
}).join(grunt.util.normalizelf('\n'));
//process
var processed = this.process(src, grunt.config('NG_VERSION'), config.strict);
if (styles) {
processedStyles = this.addStyle(processed, styles.css, styles.minify);
processed = processedStyles.js;
if (config.styles.generateCspCssFile) {
grunt.file.write(removeSuffix(config.dest) + '-csp.css', CSP_CSS_HEADER + processedStyles.css);
}
}
//write
grunt.file.write(config.dest, processed);
grunt.log.ok('File ' + config.dest + ' created.');
fn();
...

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

build: function(config, fn) {
files = this.fetch_files(config)
var src = this.concat(files)
var processed = this.process(src)
write(processed, config)
}

По мере роста кода «заголовки» становятся недостаточно заметными. И мы начинаем проявлять свое творческое начало, добавляя к нашим комментариям «украшения» – строки из дефисов, символов подчеркивания, знаков равенства и т. п. Обратите внимание на этот код из pandas:

...
# --------------- #
# dtype access #
# --------------- #
def _ensure_data(values, dtype=None):
...
def _reconstruct_data(values, dtype, original):
...
def _get_hashtable_algo(values):
...
# --------------- #
# top-level algos #
# --------------- #
def match(to_match, values, na_sentinel=-1):
...
def unique(values):
...
def isin(comps, values):
...
# --------------- #
# select n #
# --------------- #
class SelectN(object):
...
class SelectNSeries(SelectN):
...
class SelectNFrame(SelectN):
...
# ------------ #
# searchsorted #
# ------------ #
def searchsorted(arr, value, side="left", sorter=None):
...
# ---- #
# diff #
# ---- #
_diff_special = {
...
}
def diff(arr, n, axis=0):
...

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

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

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

date_acces.py:
def _ensure_data(values, dtype=None)
def _reconstruct_data(values, dtype, original)
def _get_hashtable_algo(values):
top_level_algos.py
def match(to_match, values, na_sentinel=-1):
def unique(values):
def isin(comps, values):
selectn.py
class SelectN(object):
selectn_series.py
class SelectNSeries(SelectN):
selectn_frames.py
class SelectNFrame(SelectN):
search_sorted,py
def searchsorted(arr, value, side="left", sorter=None):
diff.py
_diff_special = {
...
}
def diff(arr, n, axis=0):
...

5. /* TODO: */

Из react.js:

// TODO: decide on the top-level export form.
// This is hacky but makes it work with both Rollup and Jest
module.exports = ReactDOMServer.default || ReactDOMServer;

Не важно, как это выглядит: /* TODO */, #TODO или <! — TODO → – одно понятно: это никто никогда не сделает. Да, даже если вы рядом поместите имя ответственного. Этот человек уйдет из компании задолго до исправления этой проблемы. Мне ни разу не доводилось слышать что-то вроде: «Эй, ребята, у нас есть немного свободного времени, почему бы нам не пофиксить все «тудушки» в нашем коде?». (Если у вас есть на это время, то у вашей компании есть серьезные проблемы, более серьезные, чем все эти TODO).

Письмо будущему себе
www.xkcd.com

Главная проблема всех todo даже не в том, что это оправдание для плохо написанного кода. Дело в том, что подобные примечания запутывают читателя еще больше, заставляя обдумывать текущее состояние кода. Этот код будет вскоре изменен? Он уже исправлен, просто автор забыл удалить комментарий? Где-то висят в ожидании пул-реквесты с исправлением этой проблемы? Автор кода хотел, чтобы мы это пофиксили? Читателю приходится принимать решение: вносить изменения или смириться с последствиями.

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

Наконец, комментарии, которые нужно оставлять

Основное правило: комментарии должны отвечать на вопрос «Почему?», а код – на вопрос «Как?».

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

Использование комментариев для освещения причин, по которым вы что-то сделали определенным образом, само по себе дело хорошее, но старайтесь выражаться кратко и по теме. Если вы хотите документировать свой код, используйте wiki. Если хотите более подробно рассказать о своем решении, используйте доки. А для записи истории изменений вообще есть комментарии в git.

Хороший пример из linux:

/*
Apply the selected BCJ filter. Update *pos and s->pos to match the amount of data that got filtered. NOTE: This is implemented as a switch statement to avoid using function pointers, which could be problematic in the kernel boot code, which must avoid pointers to static data (at least on x86).
*/
static void bcj_apply(struct xz_dec_bcj *s, uint8_t *buf, size_t *pos, size_t size)

И если вы должны извлечь из этой статьи только одну умную мысль, путь это будет следующее: чтобы рассказать вашу историю, используйте код, а комментарии нужны, чтобы превращать «WTF 😨» в «Ааа…».

Спасибо за внимание! Если вам понравилось, напишите свои /*комментарии*/.

1 КОММЕНТАРИЙ

ОСТАВЬТЕ ОТВЕТ

Please enter your comment!
Please enter your name here