[PEPr] Comment on XML::XML_pQuery

From: Date: Wed, 25 Sep 2013 19:58:05 +0000
Subject: [PEPr] Comment on XML::XML_pQuery
References: 1  Groups: php.pear.dev 
Request: Send a blank email to pear-dev+get-55281@lists.php.net to get a copy of this message
1.) This appears to have the same functionality as https://github.com/symfony/CssSelector. Your API is a bit higher level but functionally they do much the same thing. The symfony package is pear installable from http://pear.symfony.com/ If you're going ahead, then the package should have "CSS Selector" in the name as that's what you're doing. 2.) The PHP_CodeStandards tool can help you make the code PEAR-compliant. 3.) constructor1, constructor2 and constructor3 methods should have better names. 4.) The load() method seems out of the scope of this package. At most, this package should accept a filename or URL and just use the default PHP commands to read the file content. Ideally I'd like to see that removed and you are just required to pass a DOMDocument. Somewhat related, isURL() won't work for all URLs. 5.) Recreating the DOM API seems crazy. Why not just return DOMNode where necessary rather than re-implementing add/remove/insert/etc? 6.) I'm not sure if __invoke() provides any benefit here. Code like $xquery = new XQuery(...); echo $xquery('my-selector')->text(); isn't easier to read than just calling $xquery->select() 7.) Use type-hinting in method signatures where possible. For example in CSS classes. 8.) A lot of your member variables are private when they should be protected to allow subclassing. -- http://pear.php.net/pepr/pepr-proposal-show.php?id=698

« previous php.pear.dev (#55281) next »