Re: svn commit: r982498 - /harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java

[Oliver Deakin <ol...@googlemail.com>]

  On 05/08/2010 09:18, hindessm@apache.org wrote:
> Author: hindessm
> Date: Thu Aug  5 08:18:50 2010
> New Revision: 982498
>
> URL: http://svn.apache.org/viewvc?rev=982498&view=rev
> Log:
> Refactor so we track firstIndex and size rather than firstIndex and lastIndex.
> I also changed ensureCapacity to calculate "minimumCapacity - array.length"
> once rather than three times.
> Still need to simplify the serialization code.
>
> Modified:
>      harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java
>
> Modified: harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java?rev=982498&r1=982497&r2=982498&view=diff
> ==============================================================================
> --- harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java (original)
> +++ harmony/enhanced/java/trunk/classlib/modules/luni/src/main/java/java/util/ArrayList.java Thu Aug  5 08:18:50 2010
> <SNIP!>
>       public void ensureCapacity(int minimumCapacity) {
> -        if (array.length<  minimumCapacity) {
> +        int required = minimumCapacity - array.length;
> +        if (required>  0) {
>               // REVIEW: Why do we check the firstIndex first? Growing
>               //         the end makes more sense
>               if (firstIndex>  0) {
> -                growAtFront(minimumCapacity - array.length);
> +                growAtFront(required);
>               } else {
> -                growAtEnd(minimumCapacity - array.length);
> +                growAtEnd(required);
>               }
>           }
>       }

This change to ensureCapacity() actually modifies its behaviour and 
looks to fix a bug in the process (perhaps accidentally?). Originally, 
if minimumCapacity was equal to array.length, we would enter the if 
block and call growAtFront()/growAtEnd() with required==0 resulting in a 
completely needless array copy and fill. With this change the check has 
been reversed but has omitted the case where required == 0, so we no 
longer do the unnecessary work. Looks like a good bug fix to me!

Regards,
Oliver