You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by lu...@apache.org on 2012/05/29 11:14:38 UTC
svn commit: r1343616 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
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
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() ^
+ Boolean.valueOf(posInf).hashCode() ^
+ Boolean.valueOf(negInf).hashCode() ^
+ Boolean.valueOf(nan).hashCode();
}
/** Get the components array.
Re: svn commit: r1343616 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Posted by James Carman <ja...@carmanconsulting.com>.
My IDE (IntelliJ) provides automatic generation of equals/hashCode.
Can't you use something like that?
On Tue, May 29, 2012 at 4:12 PM, Gilles Sadowski
<gi...@harfang.homelinux.org> wrote:
> Hi.
>
>> [...]
>> >>> --- 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.
>
> Yes, IIRC the recommended recipe in Bloch's book was something like:
> ---
> int result = 17;
> long l = Double.doubleToLongBits(field1);
> result = 37 * result + (int)(l ^ (l >>> 32));
> l = Double.doubleToLongBits(field2);
> result = 37 * result + (int)(l ^ (l >>> 32));
> return result;
> ---
>
>
> Regards,
> Gilles
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1343616 -
/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Posted by Gilles Sadowski <gi...@harfang.homelinux.org>.
Hi.
> [...]
> >>> --- 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.
Yes, IIRC the recommended recipe in Bloch's book was something like:
---
int result = 17;
long l = Double.doubleToLongBits(field1);
result = 37 * result + (int)(l ^ (l >>> 32));
l = Double.doubleToLongBits(field2);
result = 37 * result + (int)(l ^ (l >>> 32));
return result;
---
Regards,
Gilles
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1343616 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Posted by sebb <se...@gmail.com>.
On 29 May 2012 17:00, Luc Maisonobe <Lu...@free.fr> wrote:
> Hi Sebb,
>
> Le 29/05/2012 17:01, sebb a écrit :
>> On 29 May 2012 10:14, <lu...@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
Re: svn commit: r1343616 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Posted by Luc Maisonobe <Lu...@free.fr>.
Hi Sebb,
Le 29/05/2012 17:01, sebb a écrit :
> On 29 May 2012 10:14, <lu...@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.
>
>> 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 ?
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
Re: svn commit: r1343616 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/geometry/partitioning/utilities/OrderedTuple.java
Posted by sebb <se...@gmail.com>.
On 29 May 2012 10:14, <lu...@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.
> 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.
> }
>
> /** Get the components array.
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org