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

Искусство искусством, но когда бизнес меняется и через пол года год, тебя просят подправить бизнес логику в подобном выражении a=(b>3?(c<3?b+2:2):5) волосы встают дыбом! и тихо начинаешь себя ненавидеть и проклинать за подобный когда-то написанный код. :) поэтому всегда стараюсь использовать человеко понятные имена переменных и примерно как в статье агрегировать условия в более понятные и упорядочены блоки

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

А что помешает пользователю перестать быть вадидным после проверки в первой строчке и перед вызовом doSomethingWithUserId во второй строчке, в этом коде?

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

То есть если код помещается в одну строчку, то он выпоняется атомарно?

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

Проблема в том, что большинство тех, кто более или менее следуют макконнелловским принципам, на условиях сразу сползают на уровень кода в районе уроков по информатике в девятом классе. Но это субъективный опыт конечно.

если производительность не критична, то да, я тоже считаю, что как-то так и надо делать, все понятно. В противном случае можно оставить все как есть, но свести в итоге все к

  else if(isThreeCharsSpecialSymbol(tx))
  {
      return 3;
  }
  else if (isFourCharsSpecialSymbol(tx))
  {
      return 4;
  }

Или даже написать isNCharsSpecialSymbol(tx, n) и переделать все в цикл :)

Отсюда вывод: вводить переменную с говорящим названием нужно лишь тогда, когда у вас не получается написать говорящий код :-)

Верно. И еще очень часто приходится наблюдать, что когда вводят переменные с "говорящим" названием, то для нее не могут подобрать действительно говорящее и точное название.

Что говорит о том, что введение таких переменных это попытка лечить симптомы в коде, а не его архитектуры.
Если у модели или сущности нет метода IsValid(), то вынесение простыней условий в якобы "говорящие" переменные только засоряет код и ведет в side-эффектам из-за того, что часть условий вычисляется, когда это не нужно.

в данном случае да, ничем не лучше, но не все пользуют C# или что-то еще где есть оператор ?. Валидность, о которой вы пишете, должна проверяться внутри модели, валидность в данном контексте выполнения, как мне кажется, допустимо оформлять так.

спасибо, все это верно. Я просто старался примеры подобрать с использованием самых простых инструментов. Сам активно использую и C#, и JS, поэтому активно пользуюсь всеми упомянутыми вами инструментами. И user.IsValid() имеет такую же ценность как userExistsAndValid в плане code quality, особенно если у вас одно и то же понятие IsValid в десяти разных местах кода. А вот если это выливается в повсеместные if (user.IsValid() && user.lastName..., то, возможно, каждый раз стоит иметь перед глазами всю цепочку логических операций.

Я тут не пришел к вам со светочем абсолютной истины, просто вот придумал делать так, понравилось, выразил в словах и написал на Хабр :) Не думаю, что может быть какой-то однозначно "лучший" подход вообще

Вынос всех этих вычислений до цепочки, во-первых, не всегда возможен

… во-вторых, может дать проблемы с производительностью из-за вычисления кучи всего разного

И эти люди нам пытаются продать анализатор кода.

И эти люди нам пытаются продать анализатор кода.

да, это другие люди. Для меня блог pvs-studio является естественным источником кода с багами из-за дефицита code quality для статьи на Хабре. Если подскажете лучше — будет интересно взглянуть. Хотя если за эту статью мне предложат денег (бывает такое, что предлагают денег за статью постфактум?), не откажусь, честное слово :)

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

Вообще, если фанатично следовать этому правилу (хотя я сам так не делаю конечно), всегда можно переписать else if как
else { let firstConditionIsTrue = longCalculatedBoolean(); let hasSecondCondition = secondCondition != null; if (firstConditionIsTrue && hasSecondCondition ) { .... } }
возможно это и правда будет читабельней в каких-то случаях

Поэтому в таких случаях нужно выносить логику в отдельную функцию.

А можете написать, как можно первый же пример из статьи преобразовать в адекватный switch?

упс, выше уже предложили аналогичное.

наверное так или так

согласен, но пост про code quality, а не про common sense :)

у меня в изначальном варианте статьи был вариант рефакторинга из "ниче не понятно". но я не стал публиковать его по трем причинам:

1. Вы правы и опыта в плюсах у меня немного, последний раз писал активно на них лет восемь назад, а вникать заново в контекст не хотелось
2. У меня нет понимания big picture продукта и даже контекста этого метода
3. Я скорее не согласен с таким подходом к парсингу принципиально, и соглашусь с Dmitri-D, что в общем случае нужно использовать токены, но см п. 1 и особенно 2

Коллбэк в JS коде в данном случае совсем не страшен, это синхронная функция, фильтрующая уже имеющийся массив с небольшим числом элементов.

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

ну это просто мое понимание того, как должно выглядеть название поста, в который хочется зайти и написать комментарий, так-то я пишу, что все не так однозначно конечно :)

это все сведется с бесконечным const isError = ....; if(isError) { .... }. Никакой дополнительной информации вы из этого не получите, условия просты как две копейки — зачем такой формализм?

bool core_condition() {
    // мы должны тут выйти, потому-что ...
    if (in == 0) {
        return false;
    }

    // а тута...
    if (GET_CODE (in) != SUBREG) [
        return false;
    }

    // If subreg_lowpart_p is false, we cannot reload just the
	// inside since we might end up with the wrong register class
	// But if it is inside a STRICT_LOW_PART, we have
    // no choice, so we hope we do get the right register class there
    if (subreg_lowpart_p (in) == false && strict_low == false) {
        return false;
    }
	
    var subreg = SUBREG_REG(in);
    var subreg_mode = GET_MODE(subreg);

    // bla-bla
#ifdef CANNOT_CHANGE_MODE_CLASS
    if (CANNOT_CHANGE_MODE_CLASS(subreg_mode, inmode, rclass)) {
        return false;
    }
#endif
    
    // bla-bla
    if (!contains_allocatable_reg_of_mode[rclass][subreg_mode]) {
        return false;
    }
    
    // bla-bla
    if (CONSTANT_P(subreg) || GET_CODE(subreg) == PLUS) {
        return true;
    }
    
    // bla-bla
    if (strict_low) {
        return true;
    }

        // .......
        // .......
        // .......
    
#ifdef CANNOT_CHANGE_MODE_CLASS
    if (!REG_P(subreg) || REGNO(subreg) >= FIRST_PSEUDO_REGISTER) {
        return false;
    }
    
    if (!REG_CANNOT_CHANGE_MODE_P(REGNO(subreg), subreg_mode, inmode)) {
        return false;
    }
#endif

    return false;
}