https://issues.apache.org/bugzilla/show_bug.cgi?id=41959





--- Comment #9 from Andreas L. Delmelle <[EMAIL PROTECTED]>  2008-12-03 
13:39:11 PST ---
(In reply to comment #8)
> Created an attachment (id=22969)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=22969) [details]
> Proposed way to approach this problem.

Nice! Some time ago, when considering the extension to allow injecting custom
PDF dictionaries into the output, I already felt a PDFString object to be
absent. 

<snip />
> I assume Andreas could combine this with his changes. 

Yes, seems to be no problem.

> Anyway, a second pair of eyes would be
> good here. Let me know I should commit the changes.
> 
> A more critical part could be the equals() methods that I removed from many
> classes I converted. I added an equals()/hashcode() pair to PDFDictionary to
> compensate but I'm not sure if any of this is needed and if it still works as
> before.
> 

Seems more than OK at the very first glance. Certainly beneficial that the
equals()/hashCode() pair is centralized in PDFDictionary. 

For a code-compression competition, maybe equals could be:

public boolean equals(Object o) {
  return this == o
    || (o != null
      && this.getClass() == o.getClass()
      && (this.entries == ((PDFDictionary)o).entries
          || (this.entries != null 
              && this.entries.equals(((PDFDictionary)o).entries))));
}

... but this is more a personal favorite. I'm not sure whether such logical
short-circuits actually have any benefit after compilation. I just find them to
be clear and concise code-wise... :-)

One thing that could lead to issues, would be when a PDFDictionary can contain
PDFObjects that fall back on the default Object.equals(). Not sure if this is a
serious issue, but if so, fixing that for the object types in question
shouldn't be too difficult. Quickly redeclaring equals() abstract in PDFObject
revealed some 19 types that don't provide an override (hence equals() will only
return true in case of identity). 
A few of those will be taken care of by your patch. For the others, we should
probably first determine if changing them too would make sense (if they can be
used in dictionaries)
One type that concerned me, for example, is PDFName, which could act as key in
the dictionary. Then I noticed that this is implemented with Java Strings,
rather than PDFName objects, so nothing to worry about there. If a PDFName
would be used as a value, however, I think we could end up with two
dictionaries that have equal-yet-non-identical entries, so entries.equals()
would return false because one of the value pairs are of a type that invokes
Object.equals().


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

Reply via email to