On Mon, 2011-10-24 at 10:34 +0200, Mark Wielaard wrote:
> On Mon, 2011-10-24 at 10:24 +0200, Mark Wielaard wrote:
> > On Mon, 2011-10-24 at 10:11 +0300, Pekka Enberg wrote:
> > > @@ -345,7 +345,10 @@ public class HashMap<K, V> extends AbstractMap<K, V>
> > >  
> > >      while (e != null)
> > >        {
> > > -        if ((key.hashCode() == e.key.hashCode()) && equals(key, e.key))
> > > +        int hash1 = key == null ? 0 : key.hashCode();
> > > +        int hash2 = e.key == null ? 0 : e.key.hashCode();
> > > +
> > > +        if ((hash1 == hash2) && equals(key, e.key))
> > >            {
> > >              e.access(); // Must call this for bookkeeping in 
> > > LinkedHashMap.
> > >              V r = e.value;
> > 
> > Are you sure that is right?
> > What about a key which isn't null but has a hashCode() of zero?
> 
> Doh. That wouldn't be a problem, because that would still trigger the
> equals(key, e.key) to be called... OK, I think this is the right fix.

Actually, one suggestion for a micro-optimization. The compiler might
detect that hash1 only needs to be calculated once, but we can make that
explicit by pushing the int hash1 = ...; line just before the while
loop.

Thanks,

Mark

Reply via email to