On Tue, 2019-01-29 at 18:00 +0100, Matthew Brincke wrote:
> I'd like some opinions about the attached patch for solving
> issue #39 [1] in which I have opted against throwing an
> exception in order to avoid interrupting processing /Names
> arrays in PdfNamesTree::AddToDictionary(PdfObject*, PDFDictionary&)
> which would lose information.
Hi,
it sounds good.
> However, in podofopdfinfo, against which the issue is reported,
> logging (which I used instead of throwing) is disabled, so the log
> message isn't output.
I do not see any problem in having logging disabled.
> Is it OK for you if I commit a change enabling that
> logging first
No, it's not okay, there is no reason for it.
> (before committing the attached patch, which
> if there aren't objections, I plan to do tonight ca. 22:00 UTC),
Err, if I count it right, you sent a message at ~17:00 UTC and expect
people from a low traffic list residing around the globe to respond
within 5 hours? Really? Well, that's not nice of you. Especially
considering that you are not able to follow your own deadlines (this
happened multiple times here). I do not want to offend you, neither
argue about that, I only want to say that if you want an opinion, then
be patient and do not set unrealistic deadlines for no good reason.
> + // logging needs to be enabled for this (e.g. in
> podofopdfinfo)
Please, try to avoid useless comments. The developer documentation
states what the function is good for, no need to write into the code
any such comment. The less to reference single thing of all the places
where this can "strike".
> + if ( it == names.end() )
Bonus points if you make the names.end() called only once, not two-
times-the-length-of-the-array in the cycle, but it's not a question for
this change. It's for other places in the code, to do some performance
improvements.
Bye,
zyx
_______________________________________________
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users
