Skip to end of metadata
Go to start of metadata

Deprecated

This rule may be deprecated and replaced by a similar guideline.

06/28/2014 -- Version 1.0

 According to The Java Language Specification (JLS), §15.7, "Evaluation Order" [JLS 2015]:

The Java programming language guarantees that the operands of operators appear to be evaluated in a specific evaluation order, namely, from left to right.

§15.7.3, "Evaluation Respects Parentheses and Precedence" adds:

Java programming language implementations must respect the order of evaluation as indicated explicitly by parentheses and implicitly by operator precedence.

When an expression contains side effects, these two requirements can yield unexpected results. Evaluation of the operands proceeds left to right, without regard to operator precedence rules and indicative parentheses; evaluation of the operators, however, obeys precedence rules and parentheses.

Expressions must not write to memory that they subsequently read and also must not write to any memory twice. Note that memory reads and writes can occur either directly in the expression from assignments or indirectly through side effects in methods called in the expression.

Noncompliant Code Example (Order of Evaluation)

This noncompliant code example shows how side effects in expressions can lead to unanticipated outcomes. The programmer intends to write access control logic based on thresholds. Each user has a rating that must be above the threshold to be granted access. The get() method is expected to return a non-zero value for authorized users and a zero value for unauthorized users.

The programmer in this example incorrectly assumes that the rightmost subexpression is evaluated first because the * operator has a higher precedence than the + operator and because the subexpression is parenthesized. This assumption leads to the incorrect conclusion that number is assigned 0 because of the rightmost number = get() subexpression. Consequently, the test in the left-hand subexpression is expected to reject the unprivileged user because the value of number is below the threshold of 10.

However, the program grants access to the unauthorized user because evaluation of the side-effect-infested subexpressions follows the left-to-right ordering rule.

class BadPrecedence {
  public static void main(String[] args) {
    int number = 17;
    int threshold = 10;
    number = (number > threshold ? 0 : -2) 
             + ((31 * ++number) * (number = get()));
    // ... 
    if (number == 0) {
      System.out.println("Access granted");
    } else {
      System.out.println("Denied access"); // number = -2
    }
  }

  public static int get() {
    int number = 0;
    // Assign number to nonzero value if authorized, else 0
    return number;
  }
}

Noncompliant Code Example (Order of Evaluation)

This noncompliant code example reorders the previous expression so that the left-to-right evaluation order of the operands corresponds with the programmer's intent. Although this code performs as expected, it still represents poor practice by writing to number three times in a single expression.

number = ((31 * ++number) * (number=get())) + (number > threshold ? 0 : -2);

Compliant Solution (Order of Evaluation)

This compliant solution uses equivalent code with no side effects and performs not more than one write per expression. The resulting expression can be reordered without concern for the evaluation order of the component expressions, making the code easier to understand and maintain.

final int authnum = get();
number = ((31 * (number + 1)) * authnum) + (authnum > threshold ? 0 : -2);

Exceptions

EXP05-J-EX0: The increment and decrement operators (++) and (--) read a numeric variable, and then assign a new value to the variable. Although these operators read and modify a value, they are well-understood and are an exception to this rule. This exception does not apply if a value modified by an increment or decrement operator is subsequently read or written.

EXP05-J-EX1: The conditional-or || and conditional-and && operators have well-understood semantics. Writes followed by subsequent writes or reads do not violate this rule if they occur in different operands of || or &&. Consider the following code example:

public void exampleMethod(InputStream in) {
  int i;
  // Process chars until '' found
  while ((i = in.read()) != -1 && i != '\'' && 
         (i = in.read()) != -1 && i != '\'') {
    // ...
  }

}

This rule is not violated by the controlling expression of the while loop because the rule is not violated by any operand to the conditional-and && operators. The subexpressions (i = in.read()) != -1 have one assignment and one side effect (the reading of a character from in).

Risk Assessment

Failure to understand the evaluation order of expressions containing side effects can result in unexpected output.

Rule

Severity

Likelihood

Remediation Cost

Priority

Level

EXP05-J

Low

Unlikely

Medium

P2

L3

Automated Detection

Detection of all expressions involving both side effects and multiple operator precedence levels is straightforward. Determining the correctness of such uses is infeasible in the general case; heuristic warnings could be useful.


Related Guidelines

Bibliography



28 Comments

  1. I recommend adding another CCE that rewrites the NCCE as an expression with no side effects. Expressions with more than 1 effect are just begging for bugs. The NCCE can also be rewritten as:

    number = ((31 * (number+1)) * get()) + (get() > threshold[0]? 0:-2);
    

    Now the expression follows the normal mathematic rules and can be safely reordered as needed.

  2. Although this solution solves the problem, in general it is advisable to avoid using expressions with more than one side-effect. It is also inadvisable to depend on the left-right ordering for evaluation of side-effects because operands are evaluated in place first, and then subject to laws of operator precedence and associativity.

    As an experienced C/C++ dev, I agree, and I think its reasonable to strengthen EXP30-J to state this outright. Multiple side effects in expressions is a notorious no-no in C and while it may be 'legal Java', its certainly confusing enough to fall under 'bad style'.

  3. I don't think that it is necessarily bad to write an expression statement such as:

        buff[off++] = value;
    

    On the other hand, I don't personally like code like:

        while ((value = in.read()) != -1) {
    

    Or the classic C:

        while (*t++ = *s++);
    
    1. I've edited the introduction to state it outright that expressions containing multiple side effects should be avoided. Notably, your examples contain only one side-effect. I am tempted to change the title of the guideline to "EXP30-J. Do not depend on operator precedence while using expressions containing multiple side-effects". Expressions with a single-side effect seem to be quite common. Specifically, why do you consider while ((value = in.read()) != -1) to be a bad practice?

  4. I still think this rule should be strengthened and simplified. It's a good idea, but I not sure how to enforce it. I also don't like the compliant code much...still seems confusing.

    From the code samples and the comments here, I gather that code that is considered good by the Java community (and likewise C code in the C community) has the following property: Every object in any expression is either read (multiple times) or written once. No memory is both read and written to, and no memory is written more than once in the same expression.

    I suggest we make the rule focus on that part (write-once ^ read-only), as that is enforceable and seems to be what the community is inherently enforcing.

    UPDATE: The rule needs some well-understood exceptions (the ++ & co operators do a read+write, and the logical & conditional operators have well-understood evaluation order, so we can be more lenient with them.) Any other exceptions?

  5. I think "These two requirements can be counter-intuitive when expressions contain side effects" is more like "Evaluation order derived from these two requirements can be counter-intuitive when expressions contain side effects". JLS 15.7 explains the detail of "Evaluation Order" fairly clearly with example code.

  6. Can we replace two appearances of the word "function" to "method"?

  7. as for EXP05-EX0, I corrected "postfix" to "prefix".
    but how about postfix operators, which reading the value and writing new values.
    should we revise EXP05-EX0 to mention BOTH prefix and postfix operators?

      • I rewrote the intro section wrt requirements.
      • Also did s/function/method/g as requested.
      • Also overhauled EXP05-EX0, should be clearer now.
  8. EXP05-EX1 shouldn't be an exception:

    the skipped value is not used so should not be assigned to i.

    1. This is certainly a poor example, and doesn't justify the exception.

      1. EX1 is a good exception, but could use a better CS. I've revised it to be more practical (read chars until EOF or a '' is encountered).  Maybe we should introduce the term 'controlling expression', which we uused in the analogous C rule?

        1.  I think the BNF for C uses this term, while the JLS just says "Expression".  Still, why not introduce the word if it adds some clarity?

          1. Should the ? :)  operator also be part of this exception?

            According to  15.7.2 Evaluate Operands before Operation

            The Java programming language guarantees that every operand of an operator (except the conditional operators &&, ||, and ? :) appears to be fully evaluated before any part of the operation itself is performed.

            1. EXP30-C. Do not depend on the order of evaluation for side effects lists many exceptions, including the short-circuit operators && and || and the conditional operator ?:. We could copy that text here (or as much as is applicable).

  9. The title doesn't match the description very well, in that the description also disallows writing followed by a subsequent read. 

    1. Agreed, although it matches half of the description well (smile)  Suggestions for a better title?

      1. I changed the title to something that more closely matched the description.  I'm not positive we have the right rule here.  The JLS 15.7 states:

        It is recommended that code not rely crucially on this specification. Code is usually clearer when each expression contains at most one side effect, as its outermost operation, and when code does not depend on exactly which exception arises as a  consequence of the left-to-right evaluation of expressions.

        If we follow this closely, we would might have the following two rules "Expressions must not contain more than one side effect, at its outermost operation" and maybe another rule that states "Code must not depend on which exception arises as a consequence of the left-to-right evaluation of expressions".

        1. The title is somewhat imprecise (what does 'follow' mean inside an expression?) But conciseness outranks precision in the title, so it'll do.

          JLS 15.7 would forbid code like a = b++ + c++; which is perfectly acceptable.

          We could certainly create a new rule forbidding dependence on which exception arises as a consequence of the left-to-right evaluation of expressions. I have no idea how we could enforce it though.

          1. Yes, you are right that 15.7 would forbid that and other perfectly acceptable code.  It is very strict.  Any formulation of this rule is going to forbid well defined code in the interest of readability.  I'm having trouble determining where the line is to be drawn, or if this should perhaps be a guideline instead.

            1. All the code this rule is forbidding is well-defined JAva...this rule is designed for usability (well-understood) code, rather than correct code. It should definitely be a rule, not a rec, because it is easy to enforce. I don't think drawing the line will be difficult.

              Upon further thought, the rule forbidding exception catches based on expression ordering is a no-go. Any expression that can throw two exceptions forces you to catch & handle both....the compiler will enforce that. So its a very hard rule to break.

              1. Does "Code must not depend on which exception arises as a consequence of the left-to-right evaluation of expressions" mean that that the following code is nonconforming? 

                class Test3 {
                    public static void main(String[] args) {
                        int j = 1;
                        try {
                            int i = forgetIt() / (j = 2);
                        } catch (Exception e) {
                            System.out.println(e);
                            System.out.println("Now j = " + j);
                        }
                    }
                    static int forgetIt() throws Exception {
                        throw new Exception("I'm outta here!");
                    }
                }

                 

  10. The exception for the increment and decrement operators (++) and (--) could be considered to apply to the first noncompliant code example, making it compliant.

    1. EX0 was poorly worded. Using ++ does not violate this rule per se as long as the value is neither read nor written elsewhere. I've clarified the exception.

  11. I'm having a little bit of trouble believing that reason we have EX0 is because "The conditional-or || and conditional-and && operators have well-understood short-circuit semantics.". 

    First of all, this is probably not well understood by everyone, and things that are well understood by some people, are not allowed by this rule.

    I think the real reason is "because this is a common idiom".  If that is the reason, why don't we just say so?

     

    1. 'short-cirtuit semantics' is a red-herring, I nixed it. While true, their short-circuit behavior is only distantly relevant to this rule. The ordering of subexpressions in a short-circuit oepration from left-to-right is well-known, so it is a common idiom, both in Java and in C. TBH, this rule is mainly "because that's what is acceptable in C". Yet I suspect we shouldn't be THAT honest in this rule :/

      1. It still sounds to me like we should say "because it is a common idiom".