commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From luc <...@spaceroots.org>
Subject Re: [5/6] [math] Fixed findbugs warning.
Date Wed, 04 Nov 2015 13:14:53 GMT
Le 2015-11-04 13:44, Gilles a écrit :
> On Tue, 03 Nov 2015 18:07:50 +0100, Luc Maisonobe wrote:
>> Le 03/11/2015 15:33, Gilles a écrit :
>>> On Tue, 03 Nov 2015 11:06:50 -0000, luc@apache.org wrote:
>>>> Fixed findbugs warning.
>>>> 
>>>> When defining compareTo, we should also define equals and hashcode.
>>>> 
>>>> 
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-math/repo
>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/commit/04454fc0
>>>> Tree: 
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/tree/04454fc0
>>>> Diff: 
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/diff/04454fc0
>>>> 
>>>> Branch: refs/heads/MATH_3_X
>>>> Commit: 04454fc0096d5d388e10c5b13024b2a947a4e923
>>>> Parents: b72d446
>>>> Author: Luc Maisonobe <luc@apache.org>
>>>> Authored: Tue Nov 3 11:23:46 2015 +0100
>>>> Committer: Luc Maisonobe <luc@apache.org>
>>>> Committed: Tue Nov 3 11:23:46 2015 +0100
>>>> 
>>>> 
>>>> 
>>>> ----------------------------------------------------------------------
>>>>  .../commons/math3/ml/neuralnet/MapUtils.java    | 24
>>>> +++++++++++++++++---
>>>>  1 file changed, 21 insertions(+), 3 deletions(-)
>>>> 
>>>> 
>>>> ----------------------------------------------------------------------
>>>> 
>>>> 
>>>> 
>>>> 
>>>> http://git-wip-us.apache.org/repos/asf/commons-math/blob/04454fc0/src/main/java/org/apache/commons/math3/ml/neuralnet/MapUtils.java
>>>> 
>>>> 
>>>> 
>>>> ----------------------------------------------------------------------
>>>> diff --git
>>>> a/src/main/java/org/apache/commons/math3/ml/neuralnet/MapUtils.java
>>>> b/src/main/java/org/apache/commons/math3/ml/neuralnet/MapUtils.java
>>>> index 83036c2..6ef9327 100644
>>>> --- 
>>>> a/src/main/java/org/apache/commons/math3/ml/neuralnet/MapUtils.java
>>>> +++ 
>>>> b/src/main/java/org/apache/commons/math3/ml/neuralnet/MapUtils.java
>>>> @@ -17,14 +17,15 @@
>>>> 
>>>>  package org.apache.commons.math3.ml.neuralnet;
>>>> 
>>>> -import java.util.HashMap;
>>>> +import java.util.ArrayList;
>>>>  import java.util.Collection;
>>>>  import java.util.Collections;
>>>> +import java.util.HashMap;
>>>>  import java.util.List;
>>>> -import java.util.ArrayList;
>>>> +
>>>> +import org.apache.commons.math3.exception.NoDataException;
>>>>  import org.apache.commons.math3.ml.distance.DistanceMeasure;
>>>>  import 
>>>> org.apache.commons.math3.ml.neuralnet.twod.NeuronSquareMesh2D;
>>>> -import org.apache.commons.math3.exception.NoDataException;
>>>>  import org.apache.commons.math3.util.Pair;
>>>> 
>>>>  /**
>>>> @@ -320,5 +321,22 @@ public class MapUtils {
>>>>          public int compareTo(PairNeuronDouble other) {
>>>>              return Double.compare(this.value, other.value);
>>>>          }
>>>> +
>>>> +        /** {@inheritDoc} */
>>>> +        @Override
>>>> +        public boolean equals(Object other) {
>>>> +            if (!(other instanceof PairNeuronDouble)) {
>>>> +                return false;
>>>> +            }
>>>> +            return Double.doubleToRawLongBits(value) ==
>>>> +                   Double.doubleToRawLongBits(((PairNeuronDouble)
>>>> other).value);
>>>> +        }
>>>> +
>>>> +        /** {@inheritDoc} */
>>>> +        @Override
>>>> +        public int hashCode() {
>>>> +            return Double.valueOf(value).hashCode();
>>>> +        }
>>>> +
>>>>      }
>>>>  }
>>> 
>>> I think that in general, it is not correct to assume pair equality
>>> by only taking "value" into account.
>>> I also think that the default implementation provided the correct
>>> semantics (for the sole usage of sorting).
>> 
>> Sure, but as the compareTo method did only use value, the equals 
>> method
>> should be consistent with it. Otherwise, you could have compareTo 
>> return
>> 0 when equals does not return true for example.
> 
> It is plain wrong (in general) to consider two objects equal just 
> because they
> have the same rank according to an ordering based on part of their 
> properties.
> [It's akin to saying e.g. that two books are the same, because they 
> happen
> to have the same price tag.]
> 
> But it was perhaps a bad design choice to have "PairNeuronDouble" 
> implement
> "Comparable".
> I propose to use an explicit "Comparator" in order to fix the problem.

Good idea!
I have done it and pushed it on both master and MATH_3_X branches.

best regards,
Luc

> 
> Best,
> Gilles
> 
> 
>> best regards,
>> Luc
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message