Парочка мелких антипаттернов, которые просачиваются на прод и об которые можно споткнуться. Оба шаблона связаны с оптимизацией условий и переписыванием их в более удобочитаемом виде: Замена продолжительных условий и Перегрузка switch (true).
Замены продолжительных условий
Я наблюдал взлёт и падение этого подхода, который сам придумал, и, возможно, приучил нескольких разработчиков писать так же.
Смысл в том, чтобы выровнять длинный запутанный список условий, например, для if:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 | // Было if ( (dict.neededProp || array.length) && isExternalFlag && (shouldRenderNestedComponent(props, flags) || shouldIgnoreExternalFlags(props, flags)) && hasMoreLinks(props, flags) ) // Стало if ([ dict.neededProp || array.length, isExternalFlag, shouldRenderNestedComponent(props, flags) || shouldIgnoreExternalFlags(props, flags), hasMoreLinks(props, flags) ].every(Boolean)) |
Второй отрывок кода выглядит более читабельно и декларативно: мы видим список условий, каждое из которых должно выполниться. Меньше визуального шума: скобок и операторов &&, легче понять вложенные условия с || (это отдельный вопрос о том, что стоило бы вынести их за скобки в виде переменных с понятными именами).
Такой же подход можно применить к условиям со множеством логических или и методом some().
В чём подвох: в отличии от первоначального вида условия с логическими операторами, не произойдёт вычислений по короткой схеме, и каждый элемент списка будет предварительно вычислен, даже если это не повлияет на результат. Про Short-circuit evaluation я писал в отдельной статье.
Это может привести к неожиданным последствиям, и однажды привело, когда я, не думая, переписал условия:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 | // Было примерно так if ( array.length && array.pop().prop && shouldRenderNestedComponent && shouldRenderLinks ) // Стало if ([ array.length, array.pop().prop, // <-- ошибка: код будет выполнен независимо от предыдущего условия shouldRenderNestedComponent, shouldRenderLinks ].every(Boolean)) |
Код был одобрен и отправлен на прод, где падал при определённых условиях. Тогда я и понял, что интуитивно (но ошибочно) перенёс ленивое вычисление условий на понравившийся мне паттерн.
Подход всё ещё может быть использован в тех редких случаях, когда необходимо выполнить все операнды, независимо от результата условия, в которых они использованы. Ещё один случай, когда использовать список условий будет дёшево, это тот, где все члены массива являются уже вычисленными значениями.
Однако, я отказался от этого шаблона, и при случае, всегда его переписываю.
Перегрузка switch (true)
Об этом я прочитал в Вы не знаете JavaScript (и увидел на проде): иногда, вместо цепочки if, можно встретить switch с «перегруженным» условием:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 | // Было if (hasMoreLinks) {} else if (isExternalFlag) {} else if (shouldIgnoreExternalFlags(props)) {} else if (array.length || object.prop) {} // Стало switch (true) { case hasMoreLinks: log('ok') break case isExternalFlag: log('ok') break case shouldIgnoreExternalFlags(props): log('ok') break case array.length || object.prop: log('ok') break } // скорее всего, выше будет ещё отключенное предупреждение // линтера про отсутствие ветки `default` |
В чём подвох: switch использует строгое сравнение, эквивалентное оператору ===. Это приводит к неожиданному, на первый взгляд, поведению: код в ветке case array.length || object.prop: не выполнится, даже если массив не пустой, а свойство объекта существует, но является чем-то, кроме true.
1 2 3 4 5 | const array = [1] array.length === true // false const object = { prop: 1 } object.prop === true // false |
«Починить» поведение этой конструкции можно завернув каждое выражение в Boolean. Это кажется лишним, если в оригинальных условиях с if и так происходит неявное приведение типов.
Почему я не пользуюсь этим подходом:
- чтобы при чтении узнать и отпарсить в голове такую конструкцию, требуется какое-то дополнительное микро-время
- чтобы правильно написать такой
switch, нужно дополнительно позаботиться о каждом условии и убедиться, что сравниваются только логические значения - это добавляет слишком много визуального шума и махинаций с веткой
default, если в исходной цепочке условий не было ветки для кода «по умолчанию»
