On Apr 8 2013, at 16:03 , Martin Buchholz wrote:
> Style: spaces around "="
> + private static final int DEFAULT_CAPACITY= 10;
I apologize for these minor style problems in this and my other patches. I am
really loathe to run any kind of automated source reformatting tool on JDK
source. Unfortunately there are a variety of styles sued within the JDK sources
and the value of having minimal diffs is too great to contemplate global
restyling. I am happy to correct any style or other errors I make and don't
notice myself.
> ---
>
> It's a matter of taste whether to remove the temp var oldCapacity. I believe
> it was coded intentionally.
> public void trimToSize() {
> modCount++;
> - int oldCapacity = elementData.length;
> - if (size < oldCapacity) {
> + if (size < elementData.length) {
> elementData = Arrays.copyOf(elementData, size);
> }
I assumed that oldCapacity was a remnant of a previous implementation prior to
the introduction of Arrays.copyOf(). It didn't seem to be worth retaining even
for explanatory value.
> ---
>
> Because "threshold" is a serialized field, its javadoc is part of the public
> interface of this class, and hence cannot refer to implementation details
> like EMPTY_TABLE.
> 161 /**
> 162 * The next size value at which to resize (capacity * load factor).
> If
> 163 * table == EMPTY_TABLE then this is the initial capacity at which
> the
> 164 * table will be created when inflated.
> 165 * @serial
> 166 */
> 167 int threshold;
> http://docs.oracle.com/javase/7/docs/api/serialized-form.html#java.util.HashMap
>
>
> ---
>
> Consider making roundUpToPowerOf2 public.
I assume you mean as part of some utility class like Integer? Reasonable and
increases the chances that some VM engineer will come along and optimize it.
Not sure what should be done about negative numbers.
This likely comes too late for Java 8 though. (I know I probably don't have
time).
>
> Best Implementation is not obvious. I would probably write a loop with core
> x = x & (x - 1)
> until get to zero.
> 274 private static int roundUpToPowerOf2(int number) {
> 275 int rounded = number >= MAXIMUM_CAPACITY
> 276 ? MAXIMUM_CAPACITY
> 277 : (rounded = Integer.highestOneBit(number)) != 0
> 278 ? (Integer.bitCount(number) > 1) ? rounded << 1 :
> rounded
> 279 : 1;
> 280
> 281 return rounded;
> 282 }
The Integer operations are Hotspot intrinsics. This version is coded so that
there are no loops. The previous looping implementation was criticized in an
earlier review. It may be better to convert this to the Hacker's Delight 3-3
implementation
