search

sevntu-checkstyle / sevntu.checkstyle

Additional Checkstyle checks, that could be added as extension to EclipseCS plugin and maven-checkstyle-plugin, Sonar checkstyle plugin, extension for CheckStyle IDEA plugin.
http://sevntu-checkstyle.github.io/sevntu.checkstyle/
190 stars 147 forks source link

LogicConditionNeedOptimization is based on a wrong idea #1025

Open Bananeweizen opened 11 months ago

Bananeweizen commented 11 months ago

return executeEmergencyBreak() && someCondition; cannot be optimized to return someCondition && executeEmergencyBreak();, because the second piece of code will not trigger the emergency brake anymore, if someCondition is true.

Generally speaking, the re-ordering of expressions inside boolean operators is only allowed, if the short-circuit evaluation does not prohibit any side effects (that is any change of global state, like hitting the brakes) that would have occurred without the reordering.

Therefore this rule is only valid for methods that are known to have no side effect, but not for general Java methods being called.

romani commented 11 months ago

Yes, this only for non side effect methods.

We need to improve javadoc to mention this https://github.com/sevntu-checkstyle/sevntu.checkstyle/blob/c1a9944da583eaba4d3a5e691e4aad8935fd49c2/sevntu-checks/src/main/java/com/github/sevntu/checkstyle/checks/coding/LogicConditionNeedOptimizationCheck.java#L28