[PEPr] +1 for Database::DB_eXist
| From: | Philippe Jausions | Date: | Mon, 29 May 2006 17:36:45 +0000 |
| Subject: | [PEPr] +1 for Database::DB_eXist | ||
| References: | 1 | Groups: | php.pear.dev |
| Request: | Send a blank email to pear-dev+get-42667@lists.php.net to get a copy of this message | ||
Philippe Jausions (http://pear.php.net/user/jausions) has voted +1 on the proposal for
Database::DB_eXist.
Proposal information:
http://pear.php.net/pepr/pepr-proposal-show.php?id=303
Vote information:
http://pear.php.net/pepr/pepr-vote-show.php?id=303&handle=jausions
This vote is conditional. The condition is:
I think it's best to "require_once 'XML/DB/eXist.php';" in
soap.class.php instead, as you are defining constants in that file, and
someone may want to simply create a soap instance without going through
the factory().
SOAP.class.php is not a PEAR standard file. it should
be XML/DB/eXist/SOAP.php. and driver.iface.php should be
XML/DB/eXist/Interface.php
In getInstance() I would suggest to use SOAP as default value of the
$driver, and drop the strtoupper() call unless lower-case "soap" is more
common in the eXist "community". That would allow for ReST-based driver
class to be names XML_DB_eXist_ReST... Not a biggie :-)
The different types of Exceptions should be sub-classed, instead of
relying on text description. For instance:
"XML_DB_eXist_NotDriverException extends XML_DB_eXist_Exception {}"
"XML_DB_eXist_SimpleXMLExtensionMissingException extends
XML_DB_eXist_Exception {}"
It makes it easier to catch specific exceptions.
In the same category the @throws doctags should be updated to "@throws
XML_DB_eXist_Exception" instead of "@throws Exception"
I also agree that connect($username, $password) should not have default
values, unless mandated by eXist standards, as this promotes bad
security setup. You could eventually default to NULL, but "guest" sounds
like bad practice.
In SOAP...setWSDL() missing space in "if(" line
You don't need to put @author doctag in all the methods, but it's not
forbidden either ;-P
Instead of extension_loaded() use PEAR::loadExtension()
The singleton() method shouldn't return a new instance for the same
driver/options, but the same one. It is not merely a alias of getInstance().
Constants' name should be all-caps, ie XML_DB_EXIST_HIGHLIGHT_ELEMENTS,
for your own backward compatibility you could declare them as
case-insensitive if necessary. Some would prefer class constants
XML_DB_eXist::HIGHLIGHT_ELEMENTS, I just point that out without any
preference myself.
--
Sent by PEPr, the automatic proposal system at http://pear.php.net