commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1343616 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Date Tue, 29 May 2012 18:21:43 GMT
On 29 May 2012 17:00, Luc Maisonobe <Luc.Maisonobe@free.fr> wrote:
> Hi Sebb,
>
> Le 29/05/2012 17:01, sebb a écrit :
>> On 29 May 2012 10:14,  <luc@apache.org> wrote:
>>> Author: luc
>>> Date: Tue May 29 09:14:37 2012
>>> New Revision: 1343616
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1343616&view=rev
>>> Log:
>>> Use proper conversion for primitive hashcode.
>>> JIRA: MATH-793
>>
>> -1
>>
>> The fix ignores what I wrote in the JIRA.
>
> Obviously, I failed to understood your point. I thought it was the cast
> which was wrong.

That was the main problem, but I also pointed out that the conversion
was unnecessary.
Unfortunately that was not very readable as I forgot to use the {code}
markers in the JIRA issue (since added).

>>
>>> Modified:
>>>    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
>>>
>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java?rev=1343616&r1=1343615&r2=1343616&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
(original)
>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Tue May 29 09:14:37 2012
>>> @@ -301,12 +301,12 @@ public class OrderedTuple implements Com
>>>     /** {@inheritDoc} */
>>>     @Override
>>>     public int hashCode() {
>>> -        return Arrays.hashCode(components)   ^
>>> -               ((Integer) offset).hashCode() ^
>>> -               ((Integer) lsb).hashCode()    ^
>>> -               ((Boolean) posInf).hashCode() ^
>>> -               ((Boolean) negInf).hashCode() ^
>>> -               ((Boolean) nan).hashCode();
>>> +        return Arrays.hashCode(components)        ^
>>> +               Integer.valueOf(offset).hashCode() ^
>>> +               Integer.valueOf(lsb).hashCode()    ^
>>
>> As noted in the JIRA, the conversion to Integer is completely
>> unnnecessary; just use the int value.
>>
>>> +               Boolean.valueOf(posInf).hashCode() ^
>>> +               Boolean.valueOf(negInf).hashCode() ^
>>> +               Boolean.valueOf(nan).hashCode();
>>
>> Similarly here.
>
> So you suggest I use somehting like (using different constants for
> different fields just to spread more the has values):
>
>  Arrays.hashCode(components) ^ offset ^ (lsb << 7) ^
>  (posInf ?  43422 : 875342) ^ (negInf ? 86535631 : 632442767) ^
>  (nan ? 51108941 : 64234)
>
> Would this be more consistent with what you have in mind ?

Yes.

What I suggested was effectively to inline the hashCode() methods for
Integer and Boolean, i.e. to eliminate the boxing.

That would have resulted in the identical hash code as before.

However, as you point out, the original hash code was probably not
ideal in that it did not distinguish the fields.

I don't know whether XOR is the best choice for combining the fields.
LANGs HashBuilder uses multiplication and addition, which is easy to
extend to any number of fields.

> best regards,
> Luc
>
>>
>>>     }
>>>
>>>     /** Get the components array.
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> 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
>

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


Mime
View raw message