Static NumPage attribute is poor design

[GoogleCodeExporter]

This is not a bug report but a comment on the code. I am critical of line
75 of the Page class (in the trunk, revision 212)

_page = _NumPage++;

This is awful! What if I want more than one PDF? Or if later on you want to
allow pages to be inserted in the middle of PDFs? It's just really bad
design; either pages should be unaware of their number within the document
(ideal but probably impractical), or the document itself should supply the
number to the page.

(Please don't take offence ... I only bother to critique worthwhile projects :)

Original issue reported on code.google.com by [email protected] on 22 Sep 2009 at 10:01

[GoogleCodeExporter]

Also it doesn't match the numbering system used in PDF itself - it's 
zero-indexed
whereas gotoPage() etc uses one-index...

Original comment by [email protected] on 22 Sep 2009 at 10:15

[GoogleCodeExporter]

I agree with you as I figured out why my page number was increasing each pdf 
creation.
I think the best place should be the addPge() method setting the number of the 
page.

I ligtly modified the source files for this purpose.

I've first create a namespace :

package org.alivepdf.pages
{
    public namespace internalPage;
}

remove from Page class
private static var _NumPage:int = 0;

then added in Page class

internalPage function set pageNumber ( pageNb:int ):void 
{
_page = pageNb; 
}

For PDF class :
add :
import org.alivepdf.pages.internalPage;
then in addPage() method
if ( page == null ) page = new Page ( defaultOrientation, defaultUnit, 
defaultSize,
defaultRotation );

pagesReferences.push ( (3+(arrayPages.length<<1))+' 0 R' );
arrayPages.push ( currentPage = page );

use namespace internalPage;
page.pageNumber = arrayPages.length;

Original comment by [email protected] on 2 Nov 2009 at 11:56

Attachments:

• internalPage.as
• Page.as
• PDF.as

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

Hi guys,

Yes, this was awful. I added this as a temporary test in earlier builds and it 
seems like 
I forgot it.

Sorry about that!

This is fixed in 0.1.5 RC on the svn.

Thibault

Original comment by thibault.imbert on 24 Jan 2010 at 2:18