Когда-нибудь мы все писали (а некоторые и пишут) плохой код, и, надеюсь, мы все работаем над улучшением наших навыков, а не просто чтением статей вроде этой.
Зачем нам писать хороший код, а не просто производительный код?
Хотя производительность вашего продукта или сайта важна, также важно и то, как выглядит ваш код. Причиной этого является то, что не только машина читает ваш код.
Во-первых, рано или поздно вам придется перечитывать собственный код, и когда это время наступит, только хорошо написанный код поможет вам понять, что вы написали, или выяснить, как это исправить.
Во-вторых, если вы работаете в команде или сотрудничаете с другими разработчиками, то все члены команды будут читать ваш код и пытаться интерпретировать его так, как они понимают. Чтобы сделать это проще для них, важно соблюдать некие правила при названии переменных и функций, ограничивать длину каждой строки и сохранять структуру вашего кода.
Наконец, давайте рассмотрим конкретный пример.
Часть 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
всегда представляет собой узел, в котором мы сейчас находимся, вместо того, чтобы быть «каким-то» узлом.