Hi, one (maybe stupid?) question: Would it be possible to enhance the FloatEqualityCheck to test not only for equality but also for comparison (<, <=, >, >=) of doubles?
E.g. the following code
double test = 0.3 - 0.2 - 0.1;
if (test>=0.0) {
System.out.println("Bigger or equal than zero");
}
if (test < 0.0) {
System.out.println("less than zero");
}
will print “less than zero”, which is probably not what most programmers would expect. If I write
if (test < 0 || test > 0)
the Rule java:S1244 would have warned me but when I write
if (test >= 0)
I don’t get any warnings from sonar. Or do you think that it would produce too many findings? Of course I could write my own Sonar custom rule to check for those cases on my own but I thought maybe such a rule could be helpful for other people as well.
I think extending S1244 to suggest replacing <= and >= with < and > is a good idea. Since floating point operations are inaccurate, it doesn’t make sense to use <= instead of <.
Would a warning about <= have helped you avoid the problem?
sorry for my late reply, I was on holiday. I have no big experience in floating point operations, but I think that one should not use neither <= nor < with floating point numbers. E.g. the following code
double test = 0.3 - 0.2 - 0.1;
if (test < 0.0) {
System.out.println("Unexpected");
}
will surprise many developers. So a warning about <= would help a bit, but not always (as a<=b can also be expressed as !(a>b)). We are using some self written comparison methods which calculates with Math.abs() the difference between two floating point numbers and then checks that this difference is bigger than a certain threshold. In my opinion it is risky to use <, <=, > or >= with floating point numbers.
I hope you had great holiday! You’re right, direct float comparisons (<, <=, etc.) are risky, and using near-equality methods is the best practice.
When designing a rule, we’re cautious about introducing False Positives because they annoy users a lot. Unlike ==, =<, =>, the <, > operators have a lot of legitimate uses and are only dangerous if you work with them at the edge. That’s why I don’t want to include <,> in the rule. Here is a link to the ticket I created.
