Vote Details for "DB_eXist" by jausions

» Details
» Comment
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.