Плохой код #4
Что может быть хуже непонятных имен и комментариев? Непонятные операции и действия!
В этой статье примеров всего три, однако, на мой взгляд, они довольно сложные. Подумайте над каждым из них, поймите, что делает код и что хотел им выразить автор. Есть ли ошибки в логике работы программы? Если да, то укажите, какие, и опишите способ их исправления.
if(time){
quality = total / (time*(time-1));
}
let peopleLeft = total - deleted ? deleted : getDeleted();
let n = someNumber();
let length = n &~ 1;
Решение
Первый пример
if(time){
quality = total / (time*(time-1));
}
Выражение if(time)
проверяет, не равно ли time undefined, null, false, 0, пустой строке или любому другому “false-сравнимову” значению. В данном случае скорее всего имелась ввиду проверка на 0, поскольку дальше на это самое time происходит деление.
Однако что будет, если time будет равно 1? Правильно, то же самое, что и если бы time равнялось 0 - деление на 0, поскольку выражение time*(time-1)
равно 0 в обоих этих двух случаях. JavaScript при делении на 0 не бросает исключение DivisionByZero
или подобное ему, а лишь возвращает бесконечность. Однако вряд ли автор имел ввиду именно это, и скорее всего quality
должно быть равно вполне конкретному числу, а не бесконечности.
Попробуем исправить этот код.
if(time && time != 1){
quality = total / (time*(time-1));
}
Теперь деления на 0 не произойдет. Однако, что будет, если в знаменатель добавится еще один множитель?
if(time && time != 1 && time != -4){
quality = total / (time*(time-1)*(time/2 + 2));
}
Нам пришлось найти еще один корень знаменателя, который равен -4, и проверять на него. Выражения в знаменателе могут становится все сложнее, и в конечном итоге мы можем найти корень неправильно, или запутаться в уже существующих корнях. Однако, если немного поразмыслить, мы поймем, что все, что мы делаем в if(...)
это проверяем, не равен ли знаменатель нулю. Давайте попробуем выразить именно эту идею “знаменатель не равен нулю” в самом коде.
let finalTime = time*(time-1)*(time/2 + 2);
if(finalTime != 0){
quality = total / finalTfime;
}
Намного лучше. Теперь при добавлении новых множителей или слагаемых в finalTime
нам не приходится менять условия проверки на 0, ведь все, что нам нужно - это чтобы сам finalTime
не равнялся нулю. Выразив эти слова в самом коде, мы избавили его от возможных ошибок и улучшили его читабельность.
Второй пример
let peopleLeft = total - deleted ? deleted : getDeleted();
Подумаем о намерениях автора данного кода. Количество оставшихся людей равно общему количеству минус количество удаленных людей. Выглядит логично, и код вполне соответствует логике автора. Или нет?
Как вы думаете, в каком порядке будут происходить вычисления? Давайте попробуем расставить скобки, сделав все более очевидным.
let peopleLeft = ( (total) - ( (deleted) ? (deleted) : ( getDeleted() ) );
Некоторые скобки лишние, и их вполне можно убрать.
let peopleLeft = total - (deleted ? deleted : getDeleted());
Вроде бы все здорово. Однако, запустив код, мы заметим, что наша программа начала вести себя по-другому. Более того, она начала вести себя правильно! Раньше мы не замечали неправильного поведения программы из-за того что у нас отсутсвовали тесты (о них мы поговорим в главе 5).
В чем же дело? Ведь мы лишь добавили скобки, чтобы подчеркнуть наши намерения. Оказывается, дело в том, что в исходном коде у операций был не совсем тот приоритет операций, который мы ожидали. Если в нем расставить скобки так, чтобы не нарушить этот приоритет, получится совсем не то, что мы хотели.
let peopleLeft = (total - deleted) ? deleted : getDeleted();
Оказывается, у оператора ?:
приоритет меньше, чем у операции вычитания, поэтому и вычисляться он будет после него.
Стоит быть крайне аккуратным с различными вычислительным операциями, особенно, если вы точно не знаете их приоритет. Лучше поставить лишние скобки, которые подчеркнут ваши намерения, чем иметь трудноуловимые баги в вашей программе.
Стоит заметить, что если вынести итоговое количество удаленных людей в отдельную переменную, все проблемы исчезнут.
let finalDeleted = deleted ? deleted : getDeleted();
let peopleLeft = total - finalDeleted;
Или, если в дальнейшем нам не нужно старое значение переменной deleted
.
if(!deleted){
deleted = getDeleted();
}
let peopleLeft = total - deleted;
Третий пример
let n = someNumber();
let length = n &~ 1;
Проблема этого примера в том, что вообще не понятно, что в нем происходит. Если для программистов на С побитовые операции являются родными (хотя это и не избавляет от регулярных ошибок в них), то для программистов на JavaScript такие выражения могут быть затруднительны для понимания, поскольку JavaScript - язык более высокого уровня, и возиться с битами в нем приходится не часто.
По аналогии с прошлым примером, для большего понимания давайте сначала расставим скобки, чтобы показать приоритет операций.
let n = someNumber();
let length = n & (~1);
Теперь поймем, что же это за операции вообще такие.
~
означает побитовое отрицание. ~
как бы переворачивает (на английском “flip”) значение каждого бита в числе - 0 превращается в 1, 1 превращает в 0. Например, если число a
в битовом представлении выглядит как 00010111
, то ~a
будет равняться 11101000
.
&
работает уже с двумя числами. &
это побитовое умножение, или логическое И. Оно равно 1 тогда и только тогда, когда оба бита тоже равны 1, иначе оно равно 0. Для большего понимания давайте построим таблицу истинности.
| a | b | a & b
-----------------
| 0 | 0 | 0
-----------------
| 0 | 1 | 0
-----------------
| 1 | 0 | 0
-----------------
| 1 | 1 | 1
Если в числах a
и b
битов много, то эта операция применяется последовательно для каждой пары битов, стоящих на одинаковых местах. Например, если a
равняется 11001100
, а b
10110111
, то получается
11001100
10110111
& --------
10000100
Как видно, 1 получилось только там, где биты в обоих числах равны 1.
Теперь вернемся к нашему изначальному примеру. Чему равно ~1
? Если 1
равняется ...00001
(здесь количество 0 зависит от того, сколько бит JavaScript использует для хранения чисел), то ~1
будет равно ...11110
, то есть будет идти много единиц, а в конце будет 0.
Что же будет, если произвести побитовое умножение с этим числом? Из таблицы истинности, приведенной ранее, видно, что при битовом умножении на 1 всегда получается исходное число. 1&1 = 1
, 0&1 = 0
. При умножении же на 0 всегда получится 0. 1&0 = 0
, 0&0 = 0
. Получается, что исходное число останется нетронутым, кроме его последнего его бита.
abcdefgh
11111110
& --------
abcdefg0
За что отвечает этот последний бит? Если он равен 0, у нас получается какое-то четное число c
. Если он равен 1, у нас получается нечетное число c+1
.
Соответственно, если наше число из примера n
четное, оно остается без изменений, иначе, если оно нечетное, из него вычитается 1. По другому пример можно переписать так.
let n = someNumber();
let length = (n%2 == 0) ? n : n-1;
Такая запись немного длиннее, зато для ее понимания не нужно знать всю вышеописанную теорию о битах (хотя знать ее крайне полезно).
Еще лучше было бы определить для такой операции специальную функцию, и в ней творить любые оптимизации, если простой вариант нас не устраивает.
function floorToClosestEven(n){
return n &~ 1;
}
let n = someNumber();
let length = floorToClosestEven(n);
Таким образом мы спрячем оптимизированный и непонятный код в функцию, и пользователю не придется каждый раз думать, что же это за &~ 1
такое.
Пример выше не надуман, а взят из исходного Сишноого кода NodeJS. Как я уже говорил, программисты на С любят такие штуки, и в их среде такие операции более очевидны. Однако “простым смертным” разработчикам на JS следует быть осторожными с такими заумностями. Как говорится, “When writing code, don’t be stupid, but don’t be clever either” - “Когда пишешь код, не глупи, но и не умничай”.
Итог
Старайтесь писать ваш код так, чтобы он как можно понятнее передавал ваши намерения. Не бойтесь и не ленитесь поставить лишнюю скобку, объявить лишнюю переменную или функцию - ведь код всегда читается намного чаще, чем он пишется. Это очень важное правильно, о котором многие, к сожалению, забывают.
Спасибо!
Комментарии