Never write long ifs

There are a great many errors in the conditions. You can take for example any post from the PVS-studio blog , each one has errors associated with inattentive handling of the conditions. Indeed, it is not easy to discern an error in a condition if the code looks like this (example from this post ):

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;
  }
  ....
}

Such conditions are ubiquitous; I came across them in absolutely all the projects I dealt with. This post on the habr perfectly illustrates the zoo that has developed in the programming world of how “you can” write conditions for if-else and each approach is well-reasoned and cool, in general spaces rule, tabs suck and all that. The funny thing is that it begins with the words "A conditional statement in its usual form is a relatively rare source of problems", for confirmation of the opposite, I refer you again to the PVS-studio blog and your own bitter experience.

I noticed that recently when writing if-else blocks I began to use the approach everywhere, which I formalized just now and wanted to share with you. In general, it coincides with the describedin the first commentary on the aforementioned post , but more radical and with a clear idea. I use this technique in general in all conditions, no matter if they are complex or simple.

In its most general form, a rule sounds like this: if there is more than one logical operator in a condition, you need to think about refactoring it. When naming variables, choose names that match business logic, rather than formal checks that are obvious from the code .

In truth, the second is even more important. If there is something to say about the condition, in addition to what is explicitly written in it, you need to think about whether it can be expressed by the name of the variable. Compare two pseudo codes:

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

and

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

In the first case, we have a purely formal check of values, we need user.id and we check if it is there, everything can be left as is. In the second case, we need an already valid model and we explain this in the variable name through the terms of business logic.

This approach has advantages during refactoring: if the verification is needed further in the method again and / or the validity conditions are expanded (it will be necessary that the user also has an email address) - the changes will be minimal.

Let's take an example, too, about the user, but more complicated, which I came across just a few days ago and refactored in this style. It was:

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

It became:

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) {
   ....
}

Of course, there are cases when some elements of the condition need to be fulfilled only if the previous ones were successful and other exceptions, but still: if there is more than one logical operator in the condition, you need to think about refactoring it. Most of these points can be resolved, by the way, with ternary expressions or local functions, but how much more readable these tools are than long ifs, each one decides for himself.

For me, the list of exceptions also includes shortcuts, obviously, if you just need to express something like:

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

it makes no sense to introduce variables.