harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hindess <mark.hind...@googlemail.com>
Subject Re: [classlib][luni] Collections classes - code reviews and optimisations
Date Tue, 03 Aug 2010 22:45:47 GMT

In message <4C56E4F8.2040105@googlemail.com>, Oliver Deakin writes:
>   Hi all,
> A little while ago Mark suggested (not sure if it was in person or
> on the list) that it would be a good idea to take a look at our
> Collections classes and do a kind of code review, thinking about
> improvements and possible optimisations.
> To that end Mark Hindess, Cath Hope and myself sat down and took a
> close look at ArrayList earlier today. We got up to the growAtEnd()
> method, annotating the code with questions and comments as we went.
> I've opened HARMONY-6607 [1] and attached the diff with our comments
> in so everyone can see what we've come up with so far.
> If you have any further comments or ideas for the code (including
> patches for improvements) please add to the discussion here - it would
> be great to have some extra input!

We had another chat today and got a bit further.  I've updated the
review patch on JIRA.

I think there was general consensus that we should move all the
exceptions to the beginning of methods.  (And in the case of removeRange
fix the exception to be meaningful.)  I'll probably look at doing this
shortly unless someone beats me too it as it was really annoying when
reading the code.

We should also consider tracking firstIndex and size rather than firstIndex 
and lastIndex because:

 * Nearly every method uses size - most calculate it but many of them
   "forget" to use it in all cases.

 * Fewer methods use lastIndex.

 * size() is the most commonly called method making it simpler would be
   a useful side-effect

 * the resulting serialization code might be simpler

I'm still puzzled as to whether the fact that the call to growForInsert
in addAll always grows by the size of the entire collection (rather than
size - free space) is intentional or a bug.  The increment calculation
should mean we always have sufficient free space even if we only ask for
the smaller (strictly necessary) amount.  So I suspect it is a bug?


> [1] https://issues.apache.org/jira/browse/HARMONY-6607

View raw message