[PEPr] Comment on File Formats::PDF Reader

From: Date: Thu, 26 Aug 2010 14:03:35 +0000
Subject: [PEPr] Comment on File Formats::PDF Reader
References: 1  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-53730@lists.php.net to get a copy of this message
Now for general feedback. This package does provide a unique feature not provided by other PEAR packages. I'd like to see it become part of PEAR. 1.) Till's advice about running phpcs is important. 2.) should the base class be marked as abstract? 3.) echo + exit should not be used in PEAR code. You should throw an exception instead. 4.) Use of search and replacement arrays might make some of the code cleaner. I notice there are a few sections where you have half a dozen str_replace calls in a row. 5.) Your debug output always uses HTML formatting. People might be using this package in a non-web context. 6.) Create exceptions specific to your package instead of throwing generic exceptions. This will allow developers to handle errors specific to your package. 7.) The die() function is not allowed to be used in PEAR. Throw an exception instead. 8.) check for gzip before executing it. On Windows it likely won't work, for example. 9.) Can any of the character encoding stuff be made easier using mbstring or iconv extensions? If so, I'd suggest using these packages as they're likely faster and more stable. 10.) Passing parameters by reference can be dangerous and is often not needed. For example, &$PDFdecoder passed to the PDFform constructor. In PHP5, objects are not copied by default. See http://www.php.net/manual/en/language.oop5.references.php 11.) Additionally, arrays in PHP5 use a copy-on-write technique that means you usually shouldn't worry about passing arrays by reference either. 12.) Catching an exception and then throwing it again and not doing anything else is pointless. (in PDFreader). -- http://pear.php.net/pepr/pepr-proposal-show.php?id=641

« previous php.pear.dev (#53730) next »