>> 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.
