Re: [PEPr] -1 for XML::XML_RPC2

Hi Greg, Greg Beaver wrote: > Greg Beaver (http://pear.php.net/user/cellog) has voted -1 on the proposal for XML::XML_RPC2. > > Proposal information: > http://pear.php.net/pepr/pepr-proposal-show.php?id=172 > Vote information: > þŒ³f©%;³Oë > PCk´http://pear.php.net/pepr/pepr-vote-show.php?id=172&handle=cellog > > Comment: > > the code is improved from the initial proposal, but is still not good > enough. I expected some serious improvements. > > Things that need to be cut out entirely: > > 1) XML_RPC2_Value. YUCK! Let's lose this horrendous relic from XML_RPC. > It will make things MUCH slower. I am extremely disappointed that you > called for a vote prior to looking at > pear.chiaraquartet.net/Chiara_XML_RPC5. We have some pretty heavy differences when designing OO software. You are pretty much performance oriented, while I privilege cleaner design. I've run the package against the xdebug profiler, with eaccelerator enabled, and class parsing takes about 5% of a typical request processing. It's not relevant. It might be relevant for non-accelerated PHP installs, but I'd wager these aren't really interested in performance. A bytecode cache is the first step in optimizing a PHP webapp. However, I resent the -1. XML_RPC2 is driver-based. Nothing stops you from writing a new backend. In fact, I'd appreciate the help in writing an xmlrpc-extension-based backend. Logically, this would be the best backend for performance conscious users. > 1) all the @xmlrpc.* fake docblock tags. People who want to export a > class that has lots of unnecessary methods should simply define a wrapper > class the way PEAR_Server does. All the @xmlrpc.* = @xmlrpc.hidden. And the fact that the package supports the tag won't stop any user from defining the wrapper class. Moreover, I'd invite you to read back on past threads: I strongly defend that people should write a wrapper class. > 2) XML_RPC2_CallHandler is only about 10 lines of code and should be > implemented inside XML_RPC2_Server XML_RPC2_Callhandler must be as clean as possible, or else it is polluting the 'exportable' function namespace. That's why it is separate. I reinforce this concept: More classes won't necessarily introduce relevant overhead in accelerated PHP servers. > 3) XML_RPC2_Method is also unnecessary XML_RPC2_Method encapsulates docblock parsing. I'm starting to question whether you took the time to analyze the design, besides analyzing the code. If you are suggesting moving this to the XML_RPC2_Server class, along with the Callhandler code, I must ask: Why go semi-procedural? Wouldn't it be faster to go procedural all the way? > 4) all the Php/ backend code should use simplexml, not $result .= > '<methodCall>'; I'm also starting to feel you have an unfounded ill stance against this package. I've lost many hours writing and documenting this. The least I'd expect is that people don't suggest using SimpleXML to *write* XML. Even if you meant DOM, I defend that for straightforward writing of XML docs, DOM is both slower, more verbose and less clear than embedding the document. I use DOM when manipulating XML documents, where it really excels. > 5) separate classes for request/response is just bloat. Again, we're bumping against the misconception that more classes == more execution time. A Request and a Response are completely different XML documents, with different payloads and different class fields. I don't really understand how you can justify merging the two. Apart from being both XML-RPC messages (they have encode and decode methods), they share nothing more. While we're at it, why not add XML_RPC2_FaulExceptions to the bundle class? </irony> > 6) you have a curl dependency in the client, but curl is REALLY hard to > install on windows, and not installed by default. This will eliminate all > but the savviest of windows users from being able to use this class. Pre-built binaries for windows: http://curl.haxx.se/download.html#Win32 I've installed it a dozen times, and it is a piece of cake. > > Things that need to be added > 1) ways to do optional parameters without having to rely on PHP 5.1 (the > isOptional() method). Chiara_XML_RPC5 has an elegant solution. Is Chiara_XML_RPC5 proposed for inclusion into PEAR? > > In short, I'm afraid this is simply not good enough for a +1 from me. I'm > sorry our communication broke down along the way. > I'm sorry too, because trying to get your +1 cost 6 months to the proposal, and now you have given nothing but weak excuses. (Yes, I'm pissed. Sorry in advance for the tone) Cheers, Sérgio Carvalho