Совершенный код: злые однострочники

~3 минуты 208

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

Например:

// Реальный код из открытого проекта Хекслета
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));