Re: [really patch] Re: HashMap putAll/putAllInternal bug

[Bryce McKinlay]

On Friday, Sep 26, 2003, at 06:26 Pacific/Auckland, Stuart Ballard 
wrote:

> @@ -709,10 +708,10 @@
>    void putAllInternal(Map m)
>    {
>      Iterator itr = m.entrySet().iterator();
> -    int msize = m.size();
> -    size = msize;
> -    while (msize-- > 0)
> +    size = 0;
> +    while (itr.hasNext())
>        {
> +        size++;
>         Map.Entry e = (Map.Entry) itr.next();
>         Object key = e.getKey();
>         int idx = hash(key);

Hi Stuart,

size() is used here because, obviously, it is generally more efficient 
to call it once rather than calling hasNext() many times. I believe 
that the current implementation is within spec according to the 
collections documentation. If your collections are returning an 
inaccurate size() then I'd argue they are not valid implementations of 
Map.

Of course, if there are real applications out there that rely on the 
way Sun implements it, then we may have to change to using hasNext(). 
But we should consider this carefully. If we must change it, then the 
addAll/putAll implementations should change throughout the collections 
classes for consistency - not just HashMap/Hashtable. As you noticed, 
in some cases this could make things significantly less efficient.

Regards

Bryce.