RE: [PEAR-DEV] New XSLT class for PEAR

Hi Pierre-Alain, > Great! do you have the sources available somewhere ? I can > give a try to integrate it to XSLT_Wrapper. As soon as I've got it uploaded somewhere, I'll let you know. Might not be tonight now. > OO is a great word here, a simple factory to the respective backend :) I've read the code. XML_XSLT_Wrapper *is* a simple wrapper, and I'm sure it works. A couple of comments, if I may. o I'd prefer to see XML_XSLT_Wrapper::factory() take a second parameter - an array to be passed to the constructor of the backend. o What does XML_XSLT_Wrapper::Init() do? $obj doesn't appear to be in-scope, so I'd expect a PHP warning generated for that. I'm not a fan of the design of your backend baseclass and API ;-) * It's stateful. Urgh. Why? * XML_XSLT_Common::setXML() takes *two* copies of the XML, if it is passed in as a string. The parameter $options is completely unused. If the XML is passed in as a file, or as a URL, this method makes no attempt to standardise behaviour by creating a string with the contents in. The variable XML_XSLT_Common::xml can be a string, a filename, and a URL. It may save on the typing, but it's a poor quality solution. * XML_XSLT_Common::setXSL() - same comments. Plus there's lots of duplicated code in these two methods. And it should be called setXSLT(), because XSL is actually something else ;-) * XML_XSLT_Common::setParams() uses foreach(). foreach() works on a copy of the array, not the array directly. As a result, this method ends up creating two copies of each parameter - and the copy that's passed in makes three. * There's no way to unset a parameter. * Why do you need setOption() at all? Much cleaner to just pass in the options to the backend as an array into the backend's constructor. * _mkdir_p() is copied from elsewhere in PEAR. That should only be necessary if there's something wrong with the design of PEAR ;-) Food for thought, Stu --