You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2011/10/25 17:57:50 UTC
Re: svn commit: r1188682 - /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java
On 25 October 2011 15:53, <mc...@apache.org> wrote:
> Author: mcucchiara
> Date: Tue Oct 25 14:53:43 2011
> New Revision: 1188682
>
> URL: http://svn.apache.org/viewvc?rev=1188682&view=rev
> Log:
> Fixed checkstyle warning (added magic numbers)
>
> Modified:
> commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java
>
> Modified: commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java
> URL: http://svn.apache.org/viewvc/commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java?rev=1188682&r1=1188681&r2=1188682&view=diff
> ==============================================================================
> --- commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java (original)
> +++ commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java Tue Oct 25 14:53:43 2011
> @@ -48,6 +48,13 @@ import java.util.Set;
> public class IntHashMap<K extends Number, V>
> implements Map<K, V>
> {
> +
> + private static final int DEFAULT_INITIAL_CAPACITY = 101;
> +
> + private static final float DEFAULT_LOAD_FACTOR = 0.75f;
> +
> + private static final int MASK = 0x7FFFFFFF;
Why are these numbers chosen? Are there any restrictions on changing
them? (e.g. does 101 have to be odd/prime/3 digits/whatever?)
It would be helpful to have some comments - otherwise they are just
magic constants ...
> +
> private Entry table[];
>
> private int count;
> @@ -91,7 +98,8 @@ public class IntHashMap<K extends Number
> }
> while ( index-- > 0 )
> {
> - if ( ( entry = table[index] ) != null )
> + entry = table[index];
> + if ( entry != null )
> {
> return true;
> }
> @@ -198,12 +206,12 @@ public class IntHashMap<K extends Number
>
> public IntHashMap( int initialCapacity )
> {
> - this( initialCapacity, 0.75f );
> + this( initialCapacity, DEFAULT_LOAD_FACTOR );
> }
>
> public IntHashMap()
> {
> - this( 101, 0.75f );
> + this( DEFAULT_INITIAL_CAPACITY, DEFAULT_LOAD_FACTOR );
> }
>
> /*
> @@ -224,7 +232,7 @@ public class IntHashMap<K extends Number
> for ( Entry old = oldTable[i]; old != null; )
> {
> Entry e = old;
> - int index = ( e.getHash() & 0x7FFFFFFF ) % newCapacity;
> + int index = ( e.getHash() & MASK ) % newCapacity;
>
> old = old.getNext();
> e.setNext( newTable[index] );
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1188682 - /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java
Posted by Simone Tripodi <si...@apache.org>.
Hi,
>> +
>> + private static final int DEFAULT_INITIAL_CAPACITY = 101;
>> +
>> + private static final float DEFAULT_LOAD_FACTOR = 0.75f;
>> +
>> + private static final int MASK = 0x7FFFFFFF;
>
> Why are these numbers chosen? Are there any restrictions on changing
> them? (e.g. does 101 have to be odd/prime/3 digits/whatever?)
> It would be helpful to have some comments - otherwise they are just
> magic constants ...
>
I'm worried that chose was made in the OGNL early days - only Luke and
Drew can explain the reason why but I'm not sure they even joined the
ML... :)
Simo
http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1188682 - /commons/proper/ognl/trunk/src/main/java/org/apache/commons/ognl/IntHashMap.java
Posted by Maurizio Cucchiara <mc...@apache.org>.
They are legacy constants, they were cabled on code, I simply
introduced two constants.
I think that the DEFAULT_LOAD_FACTOR is the same stuff defined on the
HashMap, it represents a default threshold of 75%, after that
threshold it should expand the size of the map.
The DEFAULT_INITIAL_CAPACITY is the initial size of the map, there is
no need of primitive, odd, or whatever (though they usually are power
of 2), I think that the original developers found out that 101 was the
right trade-off.
I was thinking to replace the current value with 16 (like the HashMap
value), but I realized that the class is unused since it was replaced
with new cache approach.
Twitter :http://www.twitter.com/m_cucchiara
G+ :https://plus.google.com/107903711540963855921
Linkedin :http://www.linkedin.com/in/mauriziocucchiara
Maurizio Cucchiara
On 25 October 2011 17:57, sebb <se...@gmail.com> wrote:
> DEFAULT_LOAD_FACTOR
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org