Re: [cp-patches] RFC: Hashtable cleanup

[Roman Kennke]

Hi Tom,

I applied your suggested changes and committed the attached patch. I am
going to commit the mentioned mauve test now.

2006-01-11  Roman Kennke  <address@hidden>

        Reported by: Fridjof Siebert <address@hidden>
        * java/util/Hashtable.java
        (KEYS): Removed unneeded field.
        (VALUES): Removed unneeded field.
        (ENTRIES): Removed unneeded field.
        (keys): Return a KeyEnumerator instance.
        (elements): Returns a ValueEnumerator instance.
        (toString): Use an EntryIterator instance.
        (keySet): Return a KeyIterator instance.
        (values): Return a ValueIterator instance.
        (entrySet): Return an EntryIterator instance.
        (hashCode): Use EntryIterator instance.
        (rehash): Changed this loop to avoid redundant reads and make
        it obvious that null checking is not needed.
        (writeObject): Use EntryIterator instance.
        (HashIterator): Removed class.
        (Enumerator): Removed class.
        (EntryIterator): New class.
        (KeyIterator): New class.
        (ValueIterator): New class.
        (EntryEnumerator): New class.
        (KeyEnumerator): New class.
        (ValueEnumerator): New class.

/Roman


Am Mittwoch, den 11.01.2006, 11:19 -0700 schrieb Tom Tromey:
> >>>>> "Roman" == Roman Kennke <address@hidden> writes:
> 
> Roman> So, should I check this in?
> 
> Yes, but...
> 
> Roman> +      if (idx <= 0) /* added this test to avoid
> Roman> +                     * ArrayIndexOutOfBounds when Hashtable is
> Roman> +                     * modified concurrently, return null in this
> Roman> +                     * case.  see test
> Roman> +                     * 
> com.aicas.jamaica.testlet.bugdb.JB00310.EnumerateAndModify
> Roman> +                     * --Fridi.  
> Roman> +                     */
> 
> A few nits about this: we don't usually use long end-of-line comments,
> it would be better to have an inline comment before the test.  Also we
> don't ordinarily mention people's names or reference test cases which
> aren't in Mauve.
> 
> Could you put that test case in Mauve?  That would be best since it
> would be run by the regular regression tester.
> 
> Roman> +   * appear in the enumeration.  The spec says nothing about this, but
> Roman> +   * the "Java Class Libraries" book infers that modifications to the
> 
> This should be 'implies', not 'infers'.  This occurs in a couple
> places.
> 
> Tom

signature.asc
Description: Dies ist ein digital signierter Nachrichtenteil