Плохой код #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” - “Когда пишешь код, не глупи, но и не умничай”.

Итог

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

Спасибо!