Re: [PEPr] +1 for Images::XBM

From: 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

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