[PEPr] Comment on XML::XML_pQuery

[email protected] ("Michael Gauthier") Wed, 25 Sep 2013 20:58:05 +0100 (BST)
Newsgroups php.pear.dev
Message-ID <[email protected]>
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