Re: [PEPr] +1 for Images::XBM
| From: | Evgeny Stepanischev | Date: | Wed, 02 Mar 2005 16:08:37 +0000 |
| Subject: | Re: [PEPr] +1 for Images::XBM | ||
| References: | 1 2 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-36470@lists.php.net to get a copy of this message | ||
PJ> Philippe Jausions (http://pear.php.net/user/jausions) has
PJ> voted +1 on the proposal for Images::XBM.
PJ> This vote is conditional. The condition is:
Thank you for your vote.
PJ> Sorry for the late comments:
PJ> - Most methods name start with "image". I would recommend to
PJ> drop that. It just take longer to type.
PJ> - Some method name change recommendation
PJ> + imageSX -> getWidth
PJ> + imageSY -> getHeight
PJ> + imageLine -> drawLine (and the like drawRectangle, drawEllipse...)
PJ> + imageSetPixel -> setPixel (or drawPixel for consistency)
PJ> + imageColorAt -> getColorAt
PJ> + imageXBM -> output
PJ> + imageCreateFromXBM -> createFromFile
Names of methods coincide with GD module names.
So it will be easier to user than them to remember.
PJ> - As Alan already commented:
PJ> + "if" block needs {}
PJ> + Constants needs to be IMAGE_XBM_*
Yes, thanks, I'll fix it up.
PJ> - Constructor cannot return()
I'll fix.
PJ> - In imageFigletText, if ratio is expected to be a number or
PJ> "X:Y" format, no need of preg_split(/[:x]/), just explode(':')
Ratio may be "X:Y" or "XxY" format.
PJ> - _hexdec is not needed. 0xAF is already a standard PHP
PJ> number. Just use hexdec() instead.
Hmm... Thanks! I did not know, that hexdec ('0xFF') works. Thanks
again! Live and learn...
PJ> - In imageCreateFromXBM the file resource doesn't seem to be closed in successful flow.
I have missed it, thanks!
PJ> - Add some spaces around all operators (+, =, /, *, <<...)
Okay.
PJ> - Return the result of _ellipse directly instead of calling
PJ> the method then "return true". It saves a return statement.
Ok.
PJ> - @access should be after @return in docBlock comments.
OK
PJ> - PEAR CS: Opening curly braces "{" for function declaration goes to following
PJ> line.
OK.
PJ> - Small optimization: in "for" loops, use
PJ> pre-incrementation: ++$x instead of $x++
Hmm... Thanks. It more faster than $x++?
PJ> - The save to file method doesn't seem to handle failure to
PJ> open file for writing (method always returns true.)
Yes. I'll fix it.
I'll put fixed version very soon.
--
Kind regards,
Evgeny Stepanischev