Keep if clauses side-effect free

# Keep if clauses side-effect free Source: [https://www.teamten.com/lawrence/programming/keep-if-clauses-side-effect-free.html](https://www.teamten.com/lawrence/programming/keep-if-clauses-side-effect-free.html) Avoid writing`if`clauses that have side effects: ``` if (enqueueMessage(message)) { ... } ``` The only function of an`if`statement is to test whether a condition is true\. It’s not for executing code as a side\-effect of the test\. One problem with using the return value directly, as in the above, is that the*meaning*of the returned value is unclear\. Does`enqueueMessage\(\)`return true if the message was enqueued or true if the queue is full? Make it explicit by using a variable: ``` boolean success = enqueueMessage(message); if (success) { ... } ``` The above code reads more like English: “If we were successful, …” Methods that don’t have side\-effects are \(we hope\) named so that their return value is clear, such as`isEmpty\(\)`\. This isn’t only true of boolean\-valued methods\. This code isn’t very clear: ``` if (flushQueue() == 0) { ... } ``` whereas this one is: ``` int itemsFlushed = flushQueue(); if (itemsFlushed == 0) { ... } ``` Another drawback of calling methods with side effects in`if`statements is that the entire call could be missed by a reader skimming the code\. Compare the two examples with`flushQueue\(\)`above\. In the first the reader could mistake the call for a query that returns some queue attribute\. The second more clearly has two parts: in the first an action is taken, and in the second a test is performed\. Consider this code I saw in production: ``` if (!categorySeen.add(categoryID)) continue; ``` I couldn’t figure where in the loop items were being added to the set\. I was reading that line as: ``` if (!categorySeen.contains(categoryID)) continue; ``` because I expected the contents of an`if`statement to have no side effects\. But even when I noticed the`add\(\)`I couldn’t figure out what this did\. Can you? \(According to the Javadoc of`Set`the`add\(\)`method “returns`true`if this set did not already contain the specified element”\.\) And note the extra convoluted logic because of the`continue`\(see[Avoid continue](https://www.teamten.com/lawrence/programming/avoid-continue.html)\)\. The rest of the code will run if the`categoryID`was*not**not**not*already seen: one*not*for the`continue`, one*not*for the`\!`, and one*not*as part of the API’s description\. What?\! How about: ``` boolean isNewCategory = categorySeen.add(categoryID); if (isNewCategory) { ... } ``` Here’s a dangerous combination of a method with side effects and abuse of short\-circuit evaluation: ``` if (queueNeedsFlushing() && flushQueue() == 0) { ... } ``` The second call is particularly easy to miss\. Short\-circuit evaluation was intended to protect errors in evaluating a side\-effect\-free statement, such as: ``` if (count > 0 && total/count >= MIN_AVERAGE) { ... } ``` or: ``` if (name != null && name.endsWith(".png")) { ... } ``` Don’t use the mechanism to avoid calling a method with side effects\. That’s what`if`statements were invented for: ``` if (queueNeedsFlushing()) { int itemsFlushed = flushQueue(); if (itemsFlushed == 0) { ... } } ``` You’re doing yourself and future readers harm if you think that the terse version above is better than the three\-line version here\. Three lines is a small price to pay when you’re later having a hard time following the code because you keep missing important calls to methods\. [~ See all programming posts ~](https://www.teamten.com/lawrence/programming/)