Never write long ifs

Errors in the conditions allowed a great many. For example, you can take any post from the PVS-studio blog, each has errors related to inattentive handling of conditions. Indeed, it is not easy to see the error in the condition that the code looks like this (an 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 a universal phenomenon, I came across them absolutely in all projects with which I dealt. This post on Habré perfectly illustrates the zoo established in the programmer’s world of how to “write” conditions for if-else, and each approach is well-reasoned and cool, generally spaces are driving, tabulation sucks and all that. The funny thing is that it begins with the words “The conditional operator in its usual form is a source of problems is relatively rare,” referring again to the PVS-studio blog and your own bitter experience to confirm the reverse.

I noticed that lately, when writing if-else blocks, I began to use the approach that I formalized just now and I wanted to share with you. In general, it coincides with the one described in the first comment to the above post , but more radically and with a clear idea. I use this technique in general in all conditions, no matter if they are complex or simple.
')
In the most general form, the rule is: if there is more than one logical operator in the condition, you need to think about its refactoring. When naming variables, choose names that correspond to business logic, and not formal checks that are obvious from the code .

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

 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 a valid model and we explain this in the name of the variable through terms of business logic.

This approach also has advantages in refactoring: if the verification is needed further in the method again and / or the validity conditions expand (it will be necessary for the user to have an email) - the changes will be minimal.

Let's take an example about the user too, but more difficult, which I encountered 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 its refactoring. Most of these moments can be resolved, by the way, with ternary expressions or local functions, but as far as these tools are more readable than long ifs, everyone decides for himself.

For me, shortcuts are on the list of exceptions, 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.