Hard to detect bugs and how to avoid them

Hi guys,

Today while resolving some issues I discovered some one of a kind bugs that some folks had introduced. Interestingly these bugs were introduced only because they had the best of intentions in their mind and had made the changes in their code so that it was more resilient. The language is JavaScript although most languages can be included as these were some very basic programming errors.


Here is the code snippet that was failing due to a null exception at times:

if(response != null && response.data == 1 || response.data == 3 || response.data == 5)
{
  // some logic
}

The above code looks pretty harmless at the first glance. There is a null check at the beginning on the response object and thereafter there is a check  on the value in the data property of the same response object.
Unfortunately, this code fails in case the response object is null. This is because the logical operators being used associate from left to right and the operator does not short-circuit on false. So if we break the operands for the or (||) operator:

Operand 1: response != null && response.data == 1
Operand 2: response.data == 3
Operand 3: response.data == 5

When the response object is null, the first operand evaluates to false (as the first operand of the and operator is false), since the OR operator does not short circuit on false, the second operand is evaluated and it throws a null exception as response object is null and we are trying to access the "data" property of a null object.

The proper way to implement the check will be
if(response != null && (response.data == 1 || response.data == 3 || response.data == 5))
{
  // some logic
}

This will work as the and operator will short-circuit on false, so once response object is null, the first operand evaluates to false, the result of the "and" operation will also be false.

The most readable option will be:
if(response != null)
{
   if(response.data == 1 || response.data == 3 || response.data == 5)
     {
        // some logic
     }
}

You might want to add logging here to verify as part of the else clauses that there are no "silent failures".

if(response != null)
{
   if(response.data == 1 || response.data == 3 || response.data == 5)
     {
        // some logic
     }
   else{
       console.log(" The value of response.data is not 1, 3 or 5. It is: " + response.data);
  }
}
else
{
    console.log("The response object is null.");
}