Программисты любят компактный код. Если он реализован грамотно, то такой код легко читается и не содержит частей, которые заставляют думать о нем больше, чем нужно.
Например:
// Реальный код из открытого проекта Хекслета
const programImports = source.program.body
.filter(item => item.type === 'ImportDeclaration')
.filter(item => item.source.value.startsWith('hexlet'));
// Весь код тут https://github.com/Hexlet/hexlet-exercise-kit/blob/master/import-documentation/src/index.js
Для знакомых с концепцией фильтрации он предельно понятен и достаточно хорош, чтобы не тратить время на его дальнейшую шлифовку.
Но иногда желание сделать код компактным приводит к обратному эффекту. Падает его читаемость и усложняется рефакторинг. Причем автор кода не всегда осознает это. Пока программист находится в контексте задачи, почти любое решение, каким бы плохим оно ни было, кажется ему понятным. Программист в это время держит все детали в голове, и ему легко рассуждать об этом коде. Чего не скажешь о тех, кому предстоит его читать. Более того, даже сам автор кода уже через день, два, неделю, с трудом сможет разобраться в нем.
Подписывайтесь на канал Кирилла Мокевнина в Telegram — чтобы узнать больше о программировании и профессиональном пути разработчика
Проекты Хекслета – неиссякаемый источник примеров кода для данной статьи. Я выбрал несколько наиболее интересных и ниже проведу их разбор.
// Многие программисты искренне считают, что такой код хорош,
// потому что здесь нет переменных. Но это не так.
compareFiles(JSON.parse(fs.readFileSync(fullPathToFile1)), JSON.parse(fs.readFileSync(fullPathToFile2)))
Этот пример все еще понятен и не содержит сложной логики, но уже сложен для восприятия. Почему? Из-за большого количества вложенных вычислений. Вызов функции – это сложно. Чем больше вызовов, тем сложнее их соединять между собой в голове. Проблема усугубляется тем, что не всегда очевидно чем является результат вызова. Поэтому приходится в голове держать сразу все части выражения.
Самый простой способ сделать код понятным, как это ни странно, создавать переменные (или константы) с говорящими названиями (исходим из того, что функции уже названы хорошо). Тогда наш код превратится в такой:
const config1 = JSON.parse(fs.readFileSync(fullPathToFile1));
const config2 = JSON.parse(fs.readFileSync(fullPathToFile2));
compareFiles(config1, config2);
Теперь о коде рассуждать проще, потому что большая задача превратилась в несколько маленьких. Стало понятнее, с чем мы имеем дело (это конфиги!). Как бонус, упрощается отладка, так как можно посмотреть, что лежит в промежуточных переменных. Можно ли его улучшить дальше? Да, вложенные вызовы всегда должны вызывать желание избавиться от них. Особенно если вложенные вызовы производят побочные эффекты.
Для себя я вывел правило, которое называю “правило трех вызовов”. Оно говорит о том, что нужно разбивать выражения, содержащие три и более вложенных вызова:
f(f2(f3())); // нужно разбивать
f(f2()); // вероятно не нужно (а может и нужно)
Хозяйке на заметку. Лучший способ избавиться от вложенных вызовов – воспользоваться механизмом pipeline. Благодаря babel он есть и в js.
Другой пример:
expect(genDiff(first, second, format)).toBe(fs.readFileSync(`${__dirname}${result}`, 'UTF-8').trimRight());
Это выражение уже сложнее и требует умственных усилий для анализа. Особенно вторая часть, там где toBe. Именно здесь притаились три вызова и вдобавок интерполяция.
Отрефакторим этот код:
const fixturePath = path.join(__dirname, result);
const expectedValue = fs.readFileSync(fixturePath, 'UTF-8').trimRight();
const actualValue = genDiff(first, second, format);
expect(actualValue).toBe(expectedValue);
Да, кода стало 4 строчки вместо одной, но это тот случай, когда больше означает лучше. Этот код не добавляет новой логики и не увеличивает сложность. Он делает код самодокументируемым.
Следующий пример:
state.formStatus = ((validator.isURL(value) && (state.rssFlows.some(rssFlow => rssFlow.url === value) === false)) ? 'valid' : 'invalid');
Выражение на грани фола. Его можно прочитать, но второе условие в предикате слишком сложное. Оставляю вам это на самостоятельную работу. Покажите в комментариях пример того, как бы вы разделили этот код.
Ну и последний пример:
const urls = _.flatMap(['a', 'link', 'script', 'img'], item => $(item).map((i, el) => $(el).attr(attributes[item])).get().filter(el => el[0] === '/' && el.length > 1));
Попробуйте понять в нем, что во что вложено и что из чего вызывается.
