Re: ISBN Proposal

On Fri, 05 Jan 2007 15:57:52 +0100, Lukas Feiler <lukas@empoweredmail.com> wrote:

Hi, Please always CC to pear-dev@lists.php.net. Tom Klingenberg wrote:

A couple of things I noticed: - a direct link to the source code would be nice (i.e. to a

phps file)

That should be added to the proposal. done.

[...] please consider that double quotes will make your code run slower (mostly because of the variable parsing involved). isn't that a myth? whatever i came from the single quote front and ended up double quoting finally. maybe its because it looks better?

I think the use of single quotes can be considered a best practice in PEAR. i don't know, the best practice Guidline did not mention it that way. maybe it's more important to improve other stuff first. i think I'll run PHP_CodeSniffer over the code and similar stuff first (see below) for example.

- write your own exception class that extends

PEAR_Exception and throw

that

No, this is a php5 package, please refer to: http://pear.php.net/manual/en/standars.errors.php

http://pear.php.net/manual/en/standars.errors.php#AEN1320 states (regarding PHP5 packages): "[...] each PEAR package must provide a top level exception, named <Package_Name>_Exception. It is considered best practice that the package never throws exceptions that aren't descendant from its top level exception." i'll take a closer look on that, thanks to point on it.

- IMHO most of the functionality provided by your package

could go into

Validate_ISPN; have you talked to the maintainers of Validate_ISPN?

as an answer to that question, please check the answer to phillipe i just send to the mailinglist, i wrote about the differences there. mainly Validate_ISPN is about validating an ISPN (inkl. ISBN); ISBN contains validation of an ISBN as well but that is the only intersection.

That's not really my call; let's see what the maintainers of Validate_ISPN have to say.

shure, it would be great to have their feedback. here is some more feedback on that topic: <quote> [...] Very good indeed. What might be done is to have the Structure_ISBN or Numeric_ISBN package for the conversion & version identification, and reuse that package in Validate_ISPN to do more thorough validation. Although it would probably be Validate2_ISPN instead due to PHP 4 vs. 5... </quote> anyway the message i was refering to was not posted to the list by error, but it should be available now. thanks for your feedback, tom