Skip to end of metadata
Go to start of metadata

Using the assignment operator in conditional expressions frequently indicates programmer error and can result in unexpected behavior. The assignment operator should not be used in the following contexts:

  • if  (controlling expression)
  • while (controlling expression)
  • do ... while (controlling expression)
  • for (second operand)
  • switch (controlling expression)
  • ?:  (first operand)
  • &&  (either operand)
  • ||  (either operand)
  • ?:  (second or third operands) where the ternary expression is used in any of these contexts

Noncompliant Code Example

In this noncompliant code example, the controlling expression in the if statement is an assignment expression:

public void f(boolean a, boolean b) {
  if (a = b) {
    /* ... */
  }
}

Although the programmer's intent could have been to assign b to a and test the value of the result, this usage frequently occurs when the programmer mistakenly used the assignment operator = rather than the equality operator ==.

Compliant Solution

The conditional block shown in this compliant solution executes only when a is equal to b:

public void f(boolean a, boolean b) {
  if (a == b) {
    /* ... */
  }
}

Unintended assignment of b to a cannot occur.

Compliant Solution

When the assignment is intended, this compliant solution clarifies the programmer's intent:

public void f(boolean a, boolean b) {
  if ((a = b) == true) {
    /* ... */
  }
}

Compliant Solution

It may be clearer to express the logic as an explicit assignment followed by the if condition:

public void f(boolean a, boolean b) {
  a = b;
  if (a) {
    /* ... */
  }
}

Noncompliant Code Example

In this noncompliant code example, an assignment expression appears as an operand of the && operator:

public void f(boolean a, boolean b, boolean flag) {
  while ( (a = b) && flag ) {
    /* ... */
  }
}

Because && is not a comparison operator, assignment is an illegal operand. Again, this is frequently a case of the programmer mistakenly using the assignment operator = instead of the equals operator ==.

Compliant Solution

When the assignment of b to a is unintended, this conditional block is now executed only when a is equal to b and flag is true:

public void f(boolean a, boolean b, boolean flag) {
  while ( (a == b) && flag ) {
    /* ... */
  }
}

Applicability

The use of the assignment operator in controlling conditional expressions frequently indicates programmer error and can result in unexpected behavior.

As an exception to this guideline, it is permitted to use the assignment operator in conditional expressions when the assignment is not the controlling expression (that is, the assignment is a subexpression), as shown in the following compliant solution:

public void assignNocontrol(BufferedReader reader)
    throws IOException{
  String line;
  while ((line = reader.readLine()) != null) {
    // ... Work with line
  }
}

Automated Detection

ToolVersionCheckerDescription
SonarQube6.7AssignmentInSubExpressionCheck 

 

Bibliography

[Hatton 1995]

§2.7.2, "Errors of Omission and Addition"

 


12 Comments

  1. This is a good example of a guideline that doesn't really benefit from a Summary or "Quality Tradeoffs" section.  We should probably make it an optional section and omit it in cases like this.

    Also there is  a random link at the bottom to EXP03-J. Do not use the equality operators when comparing values of boxed primitives   

  2. We did a lot of work on this for the C rule.  You might want to have a look below and see how much of this applies here.

    No assignment in conditional expressions                                                     [boolasgn]

    Rule

    The use of the assignment operator in the following context shall be diagnosed:

    • if  (controlling expression)
    • while        (controlling expression)
    • do ... while      (controlling expression)
    • for            (second operand)
    • ?:  (first operand)
    • &&  (either operand)
    • ||  (either operand)
    • comma operator (second operand) when the comma expression is used in any of these contexts
    • ?:  (second or third operands) where the ternary expression is used in any of these contexts

    Rationale

    Mistyping or erroneously using = in Boolean expressions, where == was intended, is a common cause of program error. This rule makes the presumption that any use of = was intended to be == unless the context makes it clear that such is not the case.

    Example(s)

    EXAMPLE 1 In this noncompliant example, a diagnostic is required because the expression x = y is used as the controlling expression of the while statement.

    while ( x = y ) { /* ... */ }  //  diagnostic required

    EXAMPLE 2 In this noncompliant example, a diagnostic is required because the expression x = y is used as the controlling expression of the while statement.

    do { /* ... */ } while ( foo(), x = y ) ; // diagnostic required

    EXAMPLE 3 In this compliant example, no diagnostic is required because the expression x = y is not used as the controlling expression of the while statement.

    do { /* ... */ } while ( x = y, p == q ) ; // no diagnostic required

    Exceptions

    --      EX1: Assignment is permitted where the result of the assignment is itself a parameter to a comparison expression (e.g., x == y or x != y) or relational expression and need not be diagnosed.

    EXAMPLE This example shows an acceptable use of this exception.

    if ( ( x = y )  != 0  ) { /* ... */ }

    --      EX2: Assignment is permitted where the expression consists of a single primary expression.

    EXAMPLE 1 This example shows an acceptable use of this exception.

    if ( ( x = y ) ) { /* ... */ }

    EXAMPLE 2 In this noncompliant example, a diagnostic is required because && is not a comparison or relational operator and the entire expression is not primary.

    if ( ( v = w ) && flag ) { /* ... */ }  // diagnostic required

    --      EX3: Assignment is permitted in the above contexts where it occurs in a function argument or array index.

    EXAMPLE This example shows an acceptable use of this exception.

    if ( foo( x = y ) ) { /* ... */ }
    1. I've revised the introductory text to use some of this material, and added another example.  I thought that any more examples would be overkill.

  3. Unfortunately, this rule loses a lot of its weight when translating from C to Java.  in C, it makes a lot of sense because anything can be a boolean value in C.  in java, however, there are no implicit conversions to booleans, so most "accidental" usage will be caught by the compiler.  and for the remaining cases, how often does one compare two boolean values in a boolean expression?

    not to mention, there are a variety of common idioms where this pattern is used, typically related to streams:

    while((bytesRead = stream.read()) > 0)

    while((line = reader.readLine()) != null) 

    i've also used it in expression where multiple null checks are necessary:

    someResource = null;
    if(((someHandle != null) && ((someResource = someHandle.get()) != null)) {
      // do something with someResource here...
    }

    after reading another rule, it came to my mind that the increment "++" and decrement "–" operators are frequently used in conditional expressions, which would also violate this rule.

    in short, this is a nice "learning to write code notice", but i'm not sure it merits the strength given here.

    1. Agreed, this is a far bigger problem in C than in Java, but you must admit it is still possible in Java. And more difficult to catch b/c it happens less often.

      This rule does not forbid using assignment everywhere in conditional expressions, it forbids using assignment as the top-level (eg controlling expression) of a conditional expression (contrast the 1st NCCE with the 2nd CS). I've added an exception, using your code sample, to emphasize this.

      This guideline specifically deals with 'assignment operators', and not with increment (++) or other expressions that have side effects. I would therefore conclude this rule does not forbid increment or decrement operators. (IIRC this rule originally intended to say "don't confuse = with =='.)

      1. Yes, this is better.  i can see through example what a "controlling expression" is, but maybe a simple definition/clarification at the beginning would be helpful (as i've never heard it used like this as a general term)?

        1. I agree. I added a definition for 'controlling expression' to our glossary. The term seems to come from compiler theory.

  4. The compliant solution in the last Applicability section claims the assignment is not the "controlling expression", but it is the only expression in the while test.  that doesn't make sense?

  5. I am having trouble understanding the text in applicability

    Exceptionally, it is permitted to use the assignment operator in conditional expressions when the assignment is not the controlling expression (that is, the assignment is a sub-expression),

    Even in the last NCE the assignment is in a sub-expression!

    1. Fixed the text. The assignment is not in a controlling expression; however being an operand of && is disallowed in the intro.