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