Page History

The conditional AND and OR operators (&& and ||, respectively) exhibit " short-circuit " operationbehavior. That is, the second operand is not evaluated if the result can be deduced solely by evaluating the first operand.

One should exercise Exercise caution if the operands following the first operand contain side effects. In the following code, the value of i is incremented only when i >= 0.

int i = /* initialize to user supplied value */
int max = /* initialize to maximum value */
if ( (i >= 0) && ( (i++) <= max) ) {
  /* code */
}

Although the behavior is well defined, it is not immediately obvious whether i gets incremented or not. Even in the absence of side effects, it is possible to write confusing expressions that do not execute as desired. This guideline exemplifies both these conditions.

Noncompliant Code Example

This noncompliant examples example is designed to rename a given file if it is present, perform operations on it, and then delete it. However, the renameTo() method will does not execute when the exists() method returns true, and an unsuspecting developer would incorrectly attempt to delete the fNew file instead of the original one. This problem is exacerbated by the fact that File.delete() does not throw an exception but returns an error code on failure, which is often silently ignored or perceived as unnecessary. (See EXP02-J. Do not ignore values returned by methods)

class BadRenameFile {
  public static void main(String[] args) {
    File fOriginal = new File("original.txt");
    File fNew = new File("new.txt");

    if(fOriginal.exists() || fOriginal.renameTo(fNew)) {
      // do something with fNew
      fNew.delete(); // fNew does not exist as renameTo() was not executed
    }
  }
} 

...

Noncompliant Code Example

In this This noncompliant example differs from the previous one in that, there are no side effects in the right hand side operand. Nevertheless, an unaware programmer can get caught in the short-circuit behavior of the conditional AND and OR operators. The programmer has combined two expressions in the if statement. The first checks whether the 'd' object is null and the second checks if the default security manager exists has been installed (by comparinf sm with null) depending on which the security check will be performed. This is a classic case of trying to club combine together two null checks into one statement. A conditional '&&' is used as using a conditional '||' would mean that whenever 'd' is null, the complete expression can still succeed depending on the value of sm (see the next noncompliant example). This would violate violates the invariants of d since as it is desired that operations on it be prohibited if it is null.

Unfortunately, when 'd' is equal to null as shown, the current if expression evaluates to false and the security check is not carried out as desiredexecuted.

// d = null
SecurityManager sm = System.getSecurityManager();	  
if(d != null && sm != null) {
  FilePermission perm = new FilePermission("file.dat", "read");
  sm.checkPermission(perm);
  /* do something with d */
}
/* read the file (skips security check) */

...

In this example, the programmer switches the ordering of the two subexpressions and uses a '||' operator to ensure the security check is carried out but does not realize that the second check would not be carried out is evaluated if the first one succeeds. The result is the inadvertent modification of 'd'.

SecurityManager sm = System.getSecurityManager();	  
if(sm != null || d != null) {
  FilePermission perm = new FilePermission("file.dat", "read");
  sm.checkPermission(perm);
  // do something with d
}
// read the file

...

if (data != null && i < data.length && data[i] != -1) ... 

This snippet would sequentially execute executes the subexpressions and avoid while avoiding an array indexing exception due to resulting from the checks that execute prior to the last (offending) subexpression.

Risk Assessment

...