Никогда не пишите длинных if-ов

Ошибок в условиях допускается великое множество. Можно взять для примера любой пост из блога PVS-studio, в каждом есть ошибки, связанные с невнимательным обращением с условиями. И правда, нелегко разглядеть ошибку в условии, если код выглядит так (пример из этого поста):

static int ParseNumber(const char* tx)
{
  ....
  else if (strlen(tx) >= 4 && (strncmp(tx, "%eps", 4) == 0
    || strncmp(tx, "+%pi", 4) == 0 || strncmp(tx, "-%pi", 4) == 0
    || strncmp(tx, "+Inf", 4) == 0 || strncmp(tx, "-Inf", 4) == 0
    || strncmp(tx, "+Nan", 4) == 0 || strncmp(tx, "-Nan", 4) == 0
    || strncmp(tx, "%nan", 4) == 0 || strncmp(tx, "%inf", 4) == 0
          ))
  {
      return 4;
  }
  else if (strlen(tx) >= 3
    && (strncmp(tx, "+%e", 3) == 0
     || strncmp(tx, "-%e", 3) == 0
     || strncmp(tx, "%pi", 3) == 0   // <=
     || strncmp(tx, "Nan", 3) == 0
     || strncmp(tx, "Inf", 3) == 0
     || strncmp(tx, "%pi", 3) == 0)) // <=
  {
      return 3;
  }
  ....
}

Такие условия явление повсеместное, я сталкивался с ними абсолютно во всех проектах с которыми я имел дело. Вот этот пост на хабре отлично иллюстрирует сложившийся в программистском мире зоопарк того, как «можно» писать условия для if-else и каждый подход аргументирован и по своему крут, вообще пробелы рулят, табуляция отстой и все такое. Самое смешное, что он начинается словами «Условный оператор в обычной своей форме источником проблем является сравнительно редко», за подтверждением обратного отсылаю вас, опять же, к блогу PVS-studio и вашему собственному горькому опыту.

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

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

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

if (model.user && model.user.id) {
    doSomethingWithUserId(model.user.id);
    ...
}

и

let userExistsAndValid = model.user && model.user.id;
if (userExistsAndValid) {
    doSomethingWithUser(model.user);
    ...
}

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

Такой подход имеет преимущества и при рефакторинге: если проверка поеадобится дальше в методе еще раз и/или расширятся условия валидности (нужно будет, чтобы у юзера еще и емейл был обязательно) — изменения будут минимальны.

Возьмем пример тоже про пользователя, но посложнее, с которым я столкнулся буквально несколько дней назад и отрефакторил в этом стиле. Было:

if (this.profile.firstName && this.profile.lastName &&
    (this.password ||
     !!_.find(this.serviceAccounts, a => a.provider === 'slack' && a.accountId)) {
   ....
}

Стало:

const hasSlack = !!_.find(this.serviceAccounts, a => a.provider === 'slack' && a.accountId);
const hasSomeLoginCredentials = this.password || hasSlack;
const hasPersonalData = this.profile.firstName && this.profile.lastName;
if (hasPersonalData && hasSomeLoginCredentials) {
   ....
}

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

Для меня в списке исключений стоят еще шорткаты, очевидно, если надо просто выразить что-то вроде

if (err || !result || !result.length === 0) return callback(err);

нет смысла вводить переменные.

Автор: osharper

Источник