Когда-нибудь мы все писали (а некоторые и пишут) плохой код, и, надеюсь, мы все работаем над улучшением наших навыков, а не просто чтением статей вроде этой.

Зачем нам писать хороший код, а не просто производительный код?

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

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

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

Наконец, давайте рассмотрим конкретный пример.

Часть 1: Как определить плохой код?

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

Например, взглянем на этот код:

Скриншот плохой версии функции «traverseUpUntil»

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

const traverseUpUntil = (el, f) => {

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

• Параметры функции не читаются, как слова.
• Допустим, el можно понять, поскольку такое имя обычно используется для обозначения элемента, но имя параметра f не объясняет ровным счётом ничего.
• Название функции можно прочитать так так: «переходить до тех пор, пока el не пройдет f», которое, вероятно, лучше читать как «переходить до тех пор, пока f не пройдет для el». Конечно, лучший способ сделать это — позволить функции вызываться как el.traverseUpUntil(f), но это другая проблема.

let p = el.parentNode

Это вторая строка. Снова проблема с именами, на этот раз с переменной. Если бы кто-то посмотрел на код, то, скорее всего, понял бы, что такое p. Это parentNode параметра el. Однако, что происходит, когда мы смотрим на p, используемое где-либо еще, у нас больше нет контекста, который объясняет, что это такое.

while (p.parentNode && !f(p)) {

В следующей строке основная проблема, с которой мы сталкиваемся — это непонимание того, что означает или делает !f(p), потому что «f» может означать всё, что угодно. Предполагается, что человек, читающий код, должен понимать, что !f(p) — это проверка текущего узла на удовлетворение определённому условию. Если она проходит, то цикл прерывается.

p = p.parentNode

Здесь всё понятно.

return p

Не совсем очевидно, что возвращается из-за неправильного имени переменной.

Часть 2: Давайте отрефакторим

Скриншот хорошей версии функции «traverseUpUntil»

Сначала мы изменяем имена параметров и их порядок: (el, f) => в (condition, node) =>.
Возможно, вам интересно, почему вместо «element (рус. элемент) я использовал «node» (рус. узел). Я использовал его по следующим причинам:

• Мы пишем код в терминах узлов, например .parentNode, поэтому почему бы не сделать его консистентным.
• «Node» короче, чем «element», и при этом смысл не теряется.

Затем мы переходим к именам переменных:

let parent = node

Очень важно полностью раскрыть значение вашей переменной в её имени, поэтому «p» теперь «parent» (рус. родитель). Возможно, вы также заметили, что теперь мы не начинаем с получения родительского узла node.parentNode, вместо этого получаем только узел.

Идём далее:

do {
    parent = parent.parentNode
} while (parent.parentNode && !condition(parent))

Вместо обычного цикла while я выбрал цикл do ... while. Это означает, что нам нужно каждый раз перед проверкой условия получать родительский узел, а не наоборот. Использование цикла do ... while также способствует тому, чтобы читать код, как обычный текст.

Давайте попробуем прочитать: «Присвоить родительский узел родителя родителю, пока у родителя есть родительский узел, а функция условия не возвращает true». Уже гораздо понятнее.

return parent

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

Часть 3: Упрощение кода

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

const traverseUpUntil = (condition, node) => {
    do {
        node = node.parentNode
    } while (node.parentNode && !condition(node))

    return node
}

Я просто убрал первую строку и заменил «parent» на «node». Таким образом, я пропустил ненужный шаг создания «parent» и перешёл прямо в цикл.

Но что насчёт имени переменной?

Хотя «node» не лучшее описание для этой переменной, оно удовлетворительное. Но давайте не будем останавливаться на этом, давайте переименуем её. Как насчёт «currentNode»?

const traverseUpUntil = (condition, currentNode) => {
    do {
        currentNode = currentNode.parentNode
    } while (currentNode.parentNode && !condition(currentNode))

    return currentNode
}

Так-то лучше! Теперь, когда мы читаем метод, мы знаем, что currentNode всегда представляет собой узел, в котором мы сейчас находимся, вместо того, чтобы быть «каким-то» узлом.