[PEPr] +1 for Web Services::Services_W3C_CSSValidator

David Coallier (http://pear.php.net/user/davidc) has voted +1 on the proposal for Web Services::Services_W3C_CSSValidator. Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=510 Vote information: http://pear.php.net/pepr/pepr-vote-show.php?id=510&handle=davidc Comment: Sorry I was on holidays thus could not comment :) I am +1. I would probably make a few simple changes: In Services_W3C_CSSValidator ------------------- public $validator_uri a constant Perhaps add a bit of setters and getters instead of only using setOptions, maybe some strict/controlled magic through __call, __set, __get. I would put constants (Top of class) where we can easily change node names: $element = $doc->getElementsByTagName('validity'); for instance would become $element = $doc->getElementsByTagName(SERVICES_W3C_CSSValidator_NODE_VALIDITY); I know it is the w3c and they rarely change formats or node names (Basically never) but what if... same applies for 'error' and 'warning' Overparanoid ? Maybe :) Next, $response->error[] = new Services_W3C_CSSValidator_Error($error); Should become : $response->addError(new Services_W3C_CSSValidator_Error($error)); This is not a standard but I think the interface is much easier to read afterwards if one goes over the code instead of looking at how and where the errors were added, etc etc. Same goes for warning. Again, this is personal preferences but I consider them to be common sense to me but you might not even consider them :). P.S. I think all variable setting and getting outside a class should be accessed using setters and getters. No public variable (imho) should be set in any ways. (Protected? Perhaps.. ;-)) Good work nonetheless btw :D You keep my +1 -- Sent by PEPr, the automatic proposal system at http://pear.php.net