Short story of the day

Never, I mean never write code like this:

boolean isRel1 = false, isRel2 = false;
if((isRel1 = element1 instanceof IRelation)
|| (isRel2 = element2 instanceof IRelation)){
...
}

Especially do not use such code in comparators. If not at first try, than later it will mess up things badly, as you rely on the variable, that will not be set because of the evaluation optimization.

This fact cost me three or four hours today…

A better solution (for those who look for usable code snippets):

boolean isRel1 = element1 instanceof IRelation;
boolean isRel2 = element2 instanceof IRelation;
if(isRel1 || isRel2) {
...
}

Author: Zoltán Ujhelyi

I am an Eclipse Technology Expert at IncQuery Labs Ltd. and a regular contributor of open-source projects, most importantly the VIATRA project at eclipse.org. Furthermore, I am in the process of finishing my PhD in computer science at the Budapest University of Technology and Economics focusing on analysis techniques for model queries and transformations.

2 thoughts on “Short story of the day”

  1. well, it’s a common pattern. There are state querying and state modifying operations. Don’t mix the two, as the state modifying feature of a query operation becomes a side effect, which causes problems as seen above.

  2. Well, it was not my code in original. But I am also guilty in this bug needing 3 or 4 hours, as I questioned why that code looks like that, but I left it alone.

    Only when the boolean marker variables proved incorrect in the debugger was it clean for me what has happened.

    And comparators are astonishigly hard to debug, as the only some specific calls happen, not for every pair in case of ordering…

Leave a Reply