[PEPr] Comment on Web Services::Services_Yadis

Christian Schmidt (http://pear.php.net/user/schmidt) has commented on the proposal for Web Services::Services_Yadis. Comment: It looks good. I am looking forward to getting OpenID support in PEAR. Here are a few comments from reading the code. I haven't tried it out. I suggest using DOMDocument::loadHTML() in _isMetaHttpEquiv() to make it more robust. Parsing HTML with regexps is a hack that will work in many cases, but not all (e.g. if the page uses single quotes for enclosing attribute values, if the content attribute appears before the http-equiv attribute, if it has obsolete <meta> elements removed using HTML comments tags etc.). Though it is a personal opinion, I think public methods should return DOMElements rather than SimpleXMLElements. SimpleXML makes a developer's life easier, if he knows what he is doing, but if he isn't familiar with the API, he will easily get confused (using var_dump(), is_array(), is_string(), isset() etc. often returns surprising results). SimpleXML-fans can easily convert to SimpleXML using simplexml_import_dom() (and vice versa, I know). Or the API can supply both a getSimpleXmlObject() and a getDOMElement() method for convenience. Isn't the prefixing of method names with _ a pre-PHP5 convention? The term "namespace" is used in variable and method names to refer to 1) instances of Services_Yadis_Xrds_Namespace, 2) namespace URIs, and 3) namespace prefixes. I suggest using the terms "namespaceObject" (or simple) "namespace"), "namespaceUri" and "prefix", respectively. Some methods are "@return string|null". At least in the native PHP API, the convention is that string functions return false rather than null in cases where a string cannot be returned. I don't know if PEAR conventions differ. Sometimes input to the Services_Yadis_Exception constructor is HTML encoded using htmlentities(). This breaks the convention from PEAR_Exception that the input is plain text. Can't the first half of Services_Yadis_Xrds::sortByPriority be replaced with ksort($unsorted) ? In Services_Yadis_Xri::setXri(), the following looks wrong (always false): "if (!strpos($xri, 'xri://') === 0 && ..." Services_Yadis_Xri::toCanonicalId() can be optimized slightly by adding [last()] to the XPath expression. In Services_Yadis_Xrds::_getValidXrdNodes() it is enforced that the XRDS document uses a specific prefix ("xrd") for the XRDS namespace and that the default namespace is a specific namespace. This seems too strict. Do these restrictions really apply to XRDS documents? I don't know, but that would seem like an unusual requirement for an XML document. In XML documents authors can generally choose the prefixes freely. In Services_Yadis::__construct() it is explicitly checked that the second argument is an array. Is this necessary when the type is specified in the function's parameter list? What is the purpose of allowing the user to register his own namespaces using Services_Yadis::addNamespace() or Services_Yadis::__construct() rather than letting him register them afterwords directly on the SimpleXMLElement (or DOMXPath)? The current aproach has the (purely theoretic) disadvantage that a user has to know that the prefixes "xrd" and "xrds" are reserved and must not be used. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=499 -- Sent by PEPr, the automatic proposal system at http://pear.php.net