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
| Tool | Version | Checker | Description |
|---|---|---|---|
| Parasoft Jtest | 2023.1 | CERT.EXP51.ASI | Avoid assignment within a condition |
| PVS-Studio | 7.30 | V6041 | |
| SonarQube | 9.9 | AssignmentInSubExpressionCheck |
Bibliography
§2.7.2, "Errors of Omission and Addition" |
13 Comments
Robert Seacord
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
Fred Long
Fixed.
Robert Seacord
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:
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 requiredEXAMPLE 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 requiredEXAMPLE 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 requiredExceptions
-- 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 ) ) { /* ... */ }Fred Long
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.
James Ahlborn
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.
David Svoboda
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 =='.)James Ahlborn
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)?
David Svoboda
I agree. I added a definition for 'controlling expression' to our glossary. The term seems to come from compiler theory.
James Ahlborn
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?
David Svoboda
fixed
Dhruv Mohindra
I am having trouble understanding the text in applicability
Even in the last NCE the assignment is in a sub-expression!
David Svoboda
Fixed the text. The assignment is not in a controlling expression; however being an operand of && is disallowed in the intro.
Jan Bělohoubek
The latest interesting error in my embedded code is related to the ternary operator. It shows, that it is dangerous to combine ternary result with other operations in a single expression (not only assignment). It is close to this rule (is there another appropriate rule I missed?). Please consider adding an example:
The uncompliant code example (synthetic - not from real code):
unsigned char test(unsigned char b) {unsigned char a = 0xF0;
return a | (b == 1) ? 0x0F : 0x0A;
}
The compliant code example (synthetic):
unsigned char test(unsigned char b) {unsigned char a = 0xF0;
return a | ((b == 1) ? 0x0F : 0x0A);
}