Comments for "Image_QRCode"

» Submit Your Comment
Comments are only accepted during the "Proposal" phase. This proposal is currently in the "Finished" phase.
» Comments

  • Brett Bieber  [2009-12-16 20:59 UTC]

    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.