Re: GSOC | Extending Common Lisp support

[Jamison Hope <jrh at theptrgroup dot com>]

On Jun 15, 2012, at 2:08 PM, Charles Turner wrote:

> Thanks for that Jamison.
>
> I'm confused about the seemingly different method of computing hash
> indices in Namespace.
>
> Namespace#add is computing the index as
>
> add (Symbol s)
>  index = s.hashCode() & mask;
>
> whereas remove is computing the index as
>
> remove (Symbol s)
>  name = s.getLocalPart();
>  index = name.hashCode() & mask;
>
> Surely with this tactic an empty table will most likely not remain  
> empty after:
>
> add(s);
> remove(s);
>
> due to the Symbol most likely having a different hash code to a
> resident string. Is this just a thinko, or am I missing something?

Look at how hashCode() is implemented in Symbol.java:

  public int hashCode ()
  {
    return name == null ? 0 : name.hashCode();
  }

And getLocalPart():

  public final String getLocalPart()
  {
    return name;
  }


So, for a non-null name field, s.hashCode() ==  
s.getLocalPart().hashCode().
If name is null (does this ever happen?), then the latter will throw a  
NPE
while the former will return a hashcode of 0.

It seems fairly pointless to call getLocalPart() and not just  
hashCode(),
since Namespace#remove doesn't appear to use the name for anything else,
but it's not quite a bug (unless that NPE can actually be an issue, but
what does it mean to have a symbol with no name?).

Note that Namespace#rehash uses
		String key = sym.getName();
		int hash = key.hashCode();

which again ends up being equivalent, as getName() also returns name.


This all assumes that JAXP-QName is not defined; if it is, then Symbol
inherits QName's implementation of hashCode() and I'm not sure whether
it's true for instances of javax.xml.namespace.QName that
hashCode() == getLocalPart().hashCode() == getName().hashCode(). In that
situation, it *might* be a bug after all.

I suspect that the differences are just historical; I see no reason
not to use Symbol#hashCode() consistently.


-Jamie

--
Jamison Hope
The PTR Group
www.theptrgroup.com