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) {
...
}
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.
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…