pdfbox-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrea Vacondio <andrea.vacon...@gmail.com>
Subject Re: Named destinations refactor/enhancement
Date Tue, 08 Sep 2015 10:24:45 GMT
>> 2. PDDocumentNameDictionary::getDests if it doesn't find the Dests name
>> tree it tries to find the catalog Dests and wraps it in a
>> PDDestinationNameTreeNode. I think this is wrong since the catalog Dests
>> is
>> a dictionary, not a name tree.
>> I also think this is a hidden behavior that shouldn't be there, if I ask a
>> name tree for Dests I don't expect it to return the catalog Dests behind
>> my
>> back, it should return null and it's up to me to take action in case it's
>> null and look at the catalog Dests. Same for the setter, I actually would
>> remove the catalog reference from the class or at least clearly document
>> this behavior in the javadoc.
>>
>
> hmm.... I'm undecided about this. On the one hand, PDFBox is usually low
> level. On the other hand, we do have some simplifications to make life
> easier, e.g. what you asked for (and just got) in (3).
>
> Of course "clearly document this behavior in the javadoc" I'd always
> agree. But you meant this only to the setter, didn't you


Sure I'm not against simplifications, I just think it's in the wrong place.
PDDocumentNameDictionary is the entity modeling the Names dictionary so if
I ask for Dests I'm expecting the Names dictionary Dests value, not the
catalog Dests value. On the other hand if I ask the Catalog "hey, give me
your Dests" then there I can expect that kind of logic, so IMHO it should
be moved from there or at least documented (both setter and getter) because
as it is now the PDDocumentNameDictionary::getDests might return something
that is not the Name Dictionary Dests.
Now, set aside this splitting hairs issue :), it's probably a rarely used
feature because as it is now I don't think it works, if that logic kicks
in, you get "NameTreeNode does not have "names" nor "kids" objects" because
the catalog Dests is not a name tree but it's a dictionary.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message