[PEPr] Comment on Web Services::SabreAMF

Philippe Jausions (http://pear.php.net/user/jausions) has commented on the proposal for Web Services::SabreAMF. Comment: No it doesn't :-) I think a lot of commenting should be done... (no offense.) 1. I'm not too familiar with AMF, so I'm not sure which category this would go in. The name would need to change unless, SabreAMF is a standard on its own. If it's just branding of AMF implementation then Services_AMF or Net_AMF may be better. 2. From what I understand a good deal of all this is serialization / unserialization of data and mapping AMF types to PHP types.... Sounds much like the whole discussion of where the JSON package should go. 3. Why put a single class with just constants in it? Either make them global constants, or have them part of another class. 4. I don't remember how the interface names should be declared. That topic kind of died off a while back. 5. Code is indented one too many time, it should be flush with the left hand-side margin. 6. Are DT_ and ET_ prefixes standardized in the AMF specification? Otherwise, what do they mean. 7. Some PEAR CS such as: + "foreach($data as $k=>$v)" instead of "foreach ($data as $k => $v)" + "function name(...) {" the opening curly braces should go on the following line. + one-liner if's without "{}" should be multilines with block {} 8. You shouldn't use require_once dirname(__FILE__) '../Const.php'; inclusions but require_once 'Services/AMF/Const.php'; instead. 9. What is the purpose of the SabreAMF_AMF3_Wrapper class? 10. Use HTTP_Request instead of cURL extension. It makes the package more portable (hopefully, cURL will be supported by HTTP_Request in the future.) 11. You should use PEAR_Exception instead of simply Exception. Also create sub-classes for each different exception. Read the Exception RFC (http://pear.php.net/pepr/pepr-proposal-show.php?id=132) and/or PEAR manual about this. 12. Docblock headers should really describe the methods/properties instead of just repeating the name. 13. For the server-side, I'd really like to see an implementation similar to Services_WebService and/or XML_RPC2. The best would be to try to incorporate or make AMF a compatible backend to them. Otherwise, it seems this package could be a very useful and a nice addition to PEAR. -Philippe Proposal information: http://pear.php.net/pepr/pepr-proposal-show.php?id=348 -- Sent by PEPr, the automatic proposal system at http://pear.php.net