[Mono-devel-list] [Patch] System.Collections.Generic List improvements

[David Waite]

On 6/20/05, Ben Maurer <bmaurer at ximian.com> wrote:
> On Mon, 2005-06-20 at 01:20 -0600, David Waite wrote:
> > +               static T[] EmptyArray = new T[0];
> 
> Can be `readonly'. Space between T and [].
will-do

> > +                       if (c != null)
> > +                       {
> 
> Coding style ;-). Also, since you have an else branch, make it if (c ==
> null), its a bit easier to understand.

Already reviewed and fixed coding style errors (across _everything_ I
have changed, so this shouldn't be an issue going forward)


> >                         if (size == data.Length)
> > -                               Capacity = Math.Max (Capacity * 2, DefaultCapacity);
> > -
> > +                               Capacity = GrowSize();
> 
> I'd make GrowSize set Capacity. Since this path is less likely, it'd be
> nice for it to be shorter.

I'll review whether I need GrowSize anymore. Before there was a desire
to have the growth algorithm separate so that I could pre-compute the
final size of the array when adding a range of items, but Hari's input
on this led me to think that just matching the final size is fine if
it is more than double current capacity. GrowIfNeeded will just take a
count argument, return nothing, and be the only internal Grow*
function.

> > +                       if (idx + count > size)
> > +                               throw new ArgumentException("index and count exceed length of list");
> 
> Integer overflow ;-). Read sp's blog.

Will cast to uint :)

> Space between <T> and (

I wasn't aware this was part of style, will change.

> >                         get {
> > -                               if ((uint) index >= (uint) size)
> > -                                       throw new IndexOutOfRangeException ();
> > +                               CheckIndex(index);
> >                                 return data [index];
> >                         }
> 
> We should ensure that this is inlined by doing it manually.

Hmm, just CheckIndex or CheckRange as well?