Re: [cp-patches] Patch: javax.print.attribute.HashAttributeSet fixlets

[Wolfgang Baer]

Hi Mark,

Mark Wielaard wrote:
> Hi Wolfgang,
>
> On Sun, 2005-11-13 at 20:58 +0100, Wolfgang Baer wrote:
>
> > 2005-11-13  Wolfgang Baer  <address@hidden>
> >
> >        * javax/print/attribute/HashAttributeSet.java:
> >        Added api docs to class and clarified method documentation.
> >        (toArray): Use iterator from values instead of entries.
> >        (hashCode): Compute hashcode according to specification.
> >        (get): Throw NullPointerException if category is null.
> >        (HashAttributeSet(Attribute[], Class)): Changed to allow
> >        Attribute[] to be null.
>
>
> Should a HashAttributeSet be thread-safe? In that case you will have to
> synchronize on the attributeMap while manipulating it or iterating over
> it.

There is nothing about it in the api docs and thats despite a user guide
the only available documentation of the Java Print Service API.

However I assume that it has not to be thread safe itself, as there exists
a AttributeSetUtilities class which provides static methods to create a
synchronizedView on the different AttributeSet classes.

> >   /**
> > -   * Returns the hashcode for this object.
> > -   *
> > -   * @return the hashcode
> > +   * Returns the hashcode value. The hashcode value is the sum of all hashcodes
> > +   * of the attributes contained in this set.
> > +   * 
> > +   * @return The hashcode for this attribute set.
> >    */
> >   public int hashCode()
> >   {
> > -    return attributeMap.hashCode() + interfaceName.hashCode();
> > +    int hashcode = 0;
> > +    Iterator it = attributeMap.values().iterator();
> > +    while (it.hasNext())
> > +      hashcode = hashcode + it.next().hashCode();
> > +          
> > +    return hashcode;
> >   }
>
>
> It would be better to xor (^) the hashCode() values unless this
> computation of the hashcode has been specified to use addition of
> course. 

Thats what is done in the normal HashMap implementation. The hashcode
is the sum over all entries hashcode and this is key.hashcode ^ value.hashcode.

The api doc specifies for HashAttributeSet that the hashcode should be the
sum over all entries hashcodes. So just returning attributeMap.hashCode()
would give us the above default behaviour. However the mauve tests show that at
least the "reference implementation" does only sum up the hashcodes of the
values and ignores the entries hashcode values.

In both case the hashcode contract is fullfiled. So the question is should
we follow the reference implementation or not ?

Wolfgang