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:
>> htt
p://pear.php.net/manual/en/standars.errors.php
>
> http://pear.php.net/manual/en/standars.errors.php#AEN13
20 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
--
PEAR Development Mailing List (http://pear.php.net/)
To unsubscribe, visit: http://www.php.net/unsub
.php
|