CODEWITHSUNDEEP
javadoublesonarqubeequality
Alfe
Alfe

Reputation: 59516

Checking doubles for equality and Sonar issues

We are checking the quality of our code using Sonar, and Sonar found code which compares a float or double for equality with a constant value like this:

if (x == 0.0) { … }

The value the variable is compared with (0.0) is constant, and in case the variable can be equal to this value, the value also isn't computed but only set via a constant. This is typically used to check whether a variable hasn't been set yet or is still at initialization state, e. g. -1.0 might be used for "not yet set" in cases where the value can only be positive.

So, since these values are never computed but only set from constants, the Sonar complaint is not useful for us. Only for computed values (or fractured ones which are not precisely representable as floats or doubles) a complaint about a test for equality makes sense.

The question I have now is: What is the best practice to change the code so that Sonar does not complain about this anymore?

I see several options:

  1. Extract the "test-for-unset" into a special test function; but that would only reduce the number of occurrences (to 1), not the issue in general.
  2. Mark the code for Sonar to ignore it with a special decorator. But we would like to avoid using such decorators.
  3. Hide the comparison behind sth like (0.0 <= x && x <= 0.0) or !(x != 0.0) (which currently seems to be okay for Sonar).
  4. Calling Double.doubleToRawLongBits() to compare the bits of the values like this: (Double.doubleToRawLongBits(x) != Double.doubleToRawLongBits(0.0)).
  5. Other ideas?

None of these solutions I really like and I thought, maybe, there is a better one out there I can't think of.

Upvotes: 15

Views: 14027

Answers (7)

MonsterMash
MonsterMash

Reputation: 13

I know this is a really old post, but I faced the same problem, and solved it using the Equals method instead of the "==" operator.

if (x.Equals(0.0)) { … }

In this way SonarQube will not complain.

Bye.

Upvotes: 1

Vivek
Vivek

Reputation: 1125

Suggestion : One of the good way would be using BigDecimal for checking equality/non-equality to 0:

BigDecimal balance = pojo.getBalance();//get your BigDecimal obj

0 != balance.compareTo(BigDecimal.ZERO)

Explanation :

The compareTo() function compares this BigDecimal with the specified BigDecimal. Two BigDecimal objects that are equal in value but have a different scale (like 2.0 and 2.00) are considered equal by this method. This method is provided in preference to individual methods for each of the six boolean comparison operators (<, ==, >, >=, !=, <=). The suggested idiom for performing these comparisons is: (x.compareTo(y) <op> 0), where is one of the six comparison operators.

(Thanks to SonarQube documentation)

Floating point math is imprecise because of the challenges of storing such values in a binary representation. Even worse, floating point math is not associative; push a float or a double through a series of simple mathematical operations and the answer will be different based on the order of those operation because of the rounding that takes place at each step.

Even simple floating point assignments are not simple:

float f = 0.1; // 0.100000001490116119384765625
double d = 0.1; // 0.1000000000000000055511151231257827021181583404541015625

(Results will vary based on compiler and compiler settings);

Therefore, the use of the equality (==) and inequality (!=) operators on float or double values is almost always an error. Instead the best course is to avoid floating point comparisons altogether. When that is not possible, you should consider using one of Java's float-handling Numbers such as BigDecimal which can properly handle floating point comparisons. A third option is to look not for equality but for whether the value is close enough. I.e. compare the absolute value of the difference between the stored value and the expected value against a margin of acceptable error. Note that this does not cover all cases (NaN and Infinity for instance).

Upvotes: 0

ACV
ACV

Reputation: 10560

There is also the correct way of doing this:

private static final double EPSILON = 0.000000000000001;
if (Math.abs(x) < EPSILON) {

}

Upvotes: 1

amstegraf
amstegraf

Reputation: 597

How about casting to (int) ?! That should have the same result as you want to check if it's zero.

If you have a calculation :

double a = c - b 
boolean x = (int)(a*100) == 0

Upvotes: -1

K Erlandsson
K Erlandsson

Reputation: 13696

If you only use these constants and comparisons for uninitialized values, one option is to set the fields to Double.NaN and use Double.isNaN() for comparison. E.g.:

double notYetInitialized = Double.NaN;
if (Double.isNaN(notYetInitialized)) {
        // handle uninitialized value
}

This makes (some) sense when reading the code - an unintialized value could possibly be said to "not be a number". And I can not imagine Sonar would have a problem with it.

Upvotes: 2

Mithfindel
Mithfindel

Reputation: 4716

The best option here is to mark the issue as false positive and to leave a comment. This way the issue and associated technical debt will disappear from your SonarQube instance, without polluting your code with annotations.

Upvotes: 5

Kevin Krumwiede
Kevin Krumwiede

Reputation: 10308

I would go with your second option:

Mark the code for Sonar to ignore it with a special decorator.

Don't be a slave to static code analysis tools. They're not perfect, and there's nothing wrong with telling them to shut up. My personal practice when using annotations like @SuppressLint is to include a comment explaining why I'm using it.

That said, I would create a constant so the code is more self-explanatory:

private static final double UNINITIALIZED = 0.0;
if (x == UNINITIALIZED) { … }

Upvotes: 19

Related Questions