[Poppler-bugs] [Bug 71213] Add new PDFWriter class for writing PDF with printing options

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=71213

--- Comment #11 from Adrian Johnson <ajohnson at redneon.com> ---
Created attachment 89020
  --> https://bugs.freedesktop.org/attachment.cgi?id=89020&action=edit
PDFWriter changes to address review comments

I've addressed most of the review comments and have attached a patch with the
changes. I'll wait for the other patches to be reviewed before rolling this
patch back into the original PDFWriter patch so I only have to rebase all the
patches once.

The comments that I didn't fix are:

1) 

>@@ +186,5 @@
>> +      mediaSize->y2 = paperSizes[0].width;
>> +      margins->x1 = paperSizes[0].bottom;
>> +      margins->x2 = mediaSize->y2 - paperSizes[0].top;
>> +      margins->y1 = paperSizes[0].left;
>> +      margins->y2 = mediaSize->x2 - paperSizes[0].right;
>
>This block is repeated a lot, I wonder if we could add a helper function,
> something like setSizeAndMargins(PDFRectangle *mediaSize, PDFRectangle
> *margins, double with, double height, double top, double bottom, double left, 
> double right); I could be static inline

The problem I had with this is we will have a function with six parameters.
When looking at a call to this function it will be hard to keep track which
parameter does what. In the example quoted above the parameter list will end up
being quite long so I don't see the value in turning it into a function.

2) signed vs unsigned
> > +  for (int i = 0; i < (int)paperSizes.size(); i++) {
> Why not using unsigned for i instead of casting?

I prefer to always use signed integers unless I am doing modular arithmetic or
bit manipulation. Mixing signed and unsigned can produce unexpected results. So
it is better to just use signed all the time. The cast adds an extra 5
characters but then so does replacing 'int' with 'unsigned'.

>> +   for (int pg = (int)pageRefs[0].size() - 1; pg >= 0; pg--) {
> Ditto.

If this is changed to unsigned the loop never exits.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20131111/691a2c49/attachment-0001.html>