Hi Gilles,
On Thursday 08 June 2017 03:22 AM, Gilles wrote:
> Hello.
>
> On Wed, 7 Jun 2017 23:52:59 +0530, Amey Jadiye wrote:
>> Hi,
>>
>> I was trying to run all checks on commons-number and found findbug is
>> failing in Precision.java[544] FE_FLOATING_POINT_EQUALITY
>>
>> {code}
>> case BigDecimal.ROUND_HALF_EVEN : {
>> double fraction = unscaled - Math.floor(unscaled);
>> if (fraction > 0.5) {
>> unscaled = Math.ceil(unscaled);
>> } else if (fraction < 0.5) {
>> unscaled = Math.floor(unscaled);
>> } else {
>> // The following equality test is intentional and needed
>> for rounding purposes
>> if (Math.floor(unscaled) / 2.0 ==
>> Math.floor(Math.floor(unscaled) / 2.0)) { // even // failing here.
>> //
>> unscaled = Math.floor(unscaled);
>> } else { // odd
>> unscaled = Math.ceil(unscaled);
>> }
>> }
>> break;
>> }
>> {code}
>>
>> Error is :
>> Test for floating point equality in
>> org.apache.commons.numbers.core.Precision.roundUnscaled(double, double,
>> int) [Of Concern(15), High confidence]
>>
>> Fix:
>> Replace equality check with below:
>> if (Math.abs((Math.floor(unscaled) / 2.0) -
>> (Math.floor(Math.floor(unscaled) / 2.0))) < .0000001)
>
> Why ".0000001"?
My intention was to use EPSILON, which should be the arbitrarily small
positive quantity, anyway I dropped the way I proposed which is
explained below.
>> we have couple of similar issues in code.
>> Let me know if we have better alternative, else will submit code.
>
> Why do you think that the strict equality check must be replaced
> since there is a comment indicating that it is "intentional"?
> [I mean, is there an identified problem with the code as it is
> now, apart from the "FindBugs" assertion?]
This is because doubles and floats cannot express every numerical value.
They are really using approximations to represent the value, so
recommended way is to compare difference of both values with some low
value(epsilon) OR better we use Double.compare(d1, d2) ?
There is no problem with code but I strongly wish we *must* pass "mvn
test apache-rat:check clirr:check checkstyle:check findbugs:check
javadoc:javadoc" which is not happening right now with commons-number.
I know commons-number is not released yet but there are some other minor
findbugs issue we should fix.
ex. In ComplexUtils, for couple of methods(complex2Interleaved,
interleaved2Complex) we are creating new exception but not throwing them
? seems valid bug to me.
>
> Thanks,
> Gilles
>
>>
>> Regards,
>> Amey
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
