Vasil Rangelov [2013-09-25 13:50 UTC] I think a name like "XMLQuery" might be confusing given the existence of the XQuery language, who's name is derived exactly out of "XML Query".
Maybe "XML_CSSQuery"? "XML_pQuery" (the "p" standing in for "PHP")? I don't know...
Also... khm... PEAR coding and naming standards.
Gonzalo Chumillas [2013-09-25 18:49 UTC] Thanks Vasil. I love the name XML_pQuery :) About the naming standards, I'll change the code to adapt it to the community standards. Thanks.
Michael Gauthier [2013-09-25 19:58 UTC] 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.
Gonzalo Chumillas [2013-09-26 01:58 UTC] Hi Michael.
1. The CssSelector class is used to convert css selectors to xpath expressions. This package pretends to be a replacement of the DOM extension. It simplifies the access to the XML documents. The package is inspired in the jQuery API.
2. I just discover the PHP_CodeSniffer tool. It is a great tool :) Thanks.
3. You are right. I wasn't inspired then.
4. The class pretends to avoid the use of the DOM API. No more DOMDocument, DOMElement, DOMNode, DOMNodeList etc... just pQuery objects :)
5. same as point 4.
6. You are right. This is confusing. I will remove this behavior in the next release.
7. and 8. I'll keep that in mind.
Thanks for your help.
|
