Ошибок в условиях допускается великое множество. Можно взять для примера любой пост из блога PVS-studio, в каждом есть ошибки, связанные с невнимательным обращением с условиями. И правда, нелегко разглядеть ошибку в условии, если код выглядит так (пример из этого поста):
Такие условия явление повсеместное, я сталкивался с ними абсолютно во всех проектах с которыми я имел дело. Вот этот пост на хабре отлично иллюстрирует сложившийся в программистском мире зоопарк того, как «можно» писать условия для if-else и каждый подход аргументирован и по своему крут, вообще пробелы рулят, табуляция отстой и все такое. Самое смешное, что он начинается словами «Условный оператор в обычной своей форме источником проблем является сравнительно редко», за подтверждением обратного отсылаю вас, опять же, к блогу PVS-studio и вашему собственному горькому опыту.
Я заметил, что в последнее время при написании if-else блоков начал повсеместно использовать подход, который формализовал только сейчас и захотел с вами поделиться. В общем и целом он совпадает с описанным в первом же комментарии к вышеупомянутому посту, но радикальней и с четкой идеей. Я использую такую технику вообще во всех условиях, не важно, сложные они или простые.
В самом общем виде правило звучит так: если в условии присутствует больше одного логического оператора, нужно задуматься о его рефакторинге. При именовании переменных выбирать имена, соответствующие бизнес-логике, а не формальным проверкам, которые очевидны из кода.
По правде, второе даже важнее. Если по поводу условия есть, что сказать, кроме того, что явно написано в нем самом, нужно подумать, можно ли это выразить названием переменной. Сравните два псевдокода:
и
В первом случае у нас чисто формальная проверка значений, нам нужен user.id и мы проверяем, есть ли он, все можно оставить как есть. Во втором случае нам нужна уже валидная модель и мы объясняем это в имени переменной через термины бизнес-логики.
Такой подход имеет преимущества и при рефакторинге: если проверка понадобится дальше в методе еще раз и/или расширятся условия валидности (нужно будет, чтобы у юзера еще и емейл был обязательно) — изменения будут минимальны.
Возьмем пример тоже про пользователя, но посложнее, с которым я столкнулся буквально несколько дней назад и отрефакторил в этом стиле. Было:
Стало:
Конечно есть случаи, когда какие-то элементы условия надо выполнять только, если предыдущие были успешны и другие исключения, но все же: если в условии присутствует больше одного логического оператора, нужно задуматься о его рефакторинге. Большинство таких моментов можно разрешить, кстати, тернарными выражениями или локальными функциями, но насколько эти инструменты более читабельны, чем длинные if-ы, каждый решает сам.
Для меня в списке исключений стоят еще шорткаты, очевидно, если надо просто выразить что-то вроде:
нет смысла вводить переменные.
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);
нет смысла вводить переменные.
