[PEPr] Comment on Images::Image_QRCode
| From: | Brett Bieber | Date: | Wed, 16 Dec 2009 20:59:46 +0000 |
| Subject: | [PEPr] Comment on Images::Image_QRCode | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-53131@lists.php.net to get a copy of this message | ||
A few suggestions:
* I would declare the private variables as such, and use protected if
possible.
* The constructor should be named __construct since you're requiring PHP
5.
* The first release should not be 1.0.0, maybe 0.1.0?
* Have you tried installing the package and running it? The data files
will not be placed into the same directory as the php files unless their
role is php. Just be aware of this and either use an install-time
replacement or find some other way to reference the files.
* I'm not sure if some of the $options passed to makeCode could be made
into constructor config options or not, for example, you can only set the
image type in the makeCode() and not in the constructor which seems a bit
odd since it's a member var of the class.
Also note - I've no clue if the implementation works as I have no need for
something like this yet. :-)
There are a few methods which are pretty large, but overall the code looks
pretty good with helpful inline comments and a lot of phpdoc declarations.
--
http://pear.php.net/pepr/pepr-proposal-show.php?id=623