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

Sergio Carvalho wrote:

Greg Beaver wrote:

I find this decision rather simple: if there are any non-sequential or non-numeric indices, struct is used, otherwise array is used.

It's not acceptable. I do exactly that when doing automatic encoding, but I assume it may be a bad decision. I mean, if you do this: $proxy->foo(array('bar','baz')); the call payload will carry an array. However, the server may be expecting a struct and will barf out if presented with an array. In XML_RPC2 you can workaround these cases by avoiding automatic encoding, and specifying the type itself: $proxy->foo(new XML_RPC_Value_Struct(array('bar', 'baz')));

This is still completely unnecessary :) $proxy->foo((object) array('bar', 'baz')) will automatically send a struct instead of an array, as objects are auto-converted to structs. The *only* instance where it is absolutely necessary to use an object is for base64 and datetime. Incidentally, it is also perfectly possible to use the above syntax in my implementation if the user does not prefer to use setType(), and never wishes to use the xmlrpc-epi extension, I'll show below. Your design, however, makes using an xmlrpc-epi backend much more complex than using the php backend internally. This strays from the main point: none of the complex classes are necessary to create a clear structure. Let me put it this way: which is going to be easier to work with inside the XML_RPC2 class/cleare to the end user? #1 <?php $xmlrpc->someFunction(array( 'hi' => (object) array(1,2.0,3), 'time' => array('hours' => 12, 'minutes' => 30), 'binarydata' => new Chiara_XML_RPC5_Custom($data, 'base64'))); // or $data = array( 'hi' => (object) array(1,2,3), 'time' => array('hours' => 12, 'minutes' => 30), 'binarydata' => $data); $xmlrpc->setType($data['binarydata'], 'base64'); $someFunction($data); ?> <?php $xmlrpc->someFunction(new XML_RPC2_Value_Struct( array(

      'hi' => new XML_RPC2_Value_Struct(array(new XML_RPC2_Value_Int(1), new XML_RPC2_Value_Double(2.0),new XML_RPC2_Value_Int(3))),
      'time' => new XML_RPC2_Value_Struct(array('hours' => new XML_RPC2_Value_Int(12), 'minutes' => new XML_RPC2_Value_Int(30))),
      'binarydata' => new XML_RPC2_Value_Base64($data))));

?> Now I realize the user probably would never do this, but your code converts the native PHP types into this exact structure. If used with an xmlrpc-epi backend, the second choice will have double conversion - the user would create the complex object, and then the backend would have to just convert it back into the native PHP types and use settype() on the base64. It's just bad design to do this, forget OO/procedural. By using these custom type classes for every single XML-RPC value, you are introducing an extreme element of inflexibility, a performance hit, and coding complexity.

I realize the extensions do this using setType. That approach strikes me as smelly. Why represent the concept of type using a band aid when the language provides me with a native form of representing types?

I have no problem with providing a custom object for datetime and base64 - This will work just fine and is necessary to work with non-extension based drivers.

<?php $xmlrpc = new Chiara_XML_RPC5('http://example.com/xmlrpc.php'); $arr = array(1,2,'base64'); $xmlrpc->setType($arr[2], 'base64'); $result = $xmlrpc->someFunction($arr); ?>

Isn't it much cleaner to do: <?php $xmlrpc = new XML_RPC2_Server('http://example.com/xmlrpc.php'); $xmlrpc->someFunction(1,2, new XML_RPC_Value_Base64('base64')); ?>

Perhaps it appears so on the surface - but the xmlrpc-epi driver would then be forced to iterate over the entire data structure looking for these classes, and convert them back to the original native PHP type and run settype() on them internally. The only driver that would benefit from this approach is the PHP-based driver, and the benefits are negligible. However, I *could* see this working as an option: $xmlrpc->someFunction(1,2, $xmlrpc->retrieveType($stuff, 'base64')); This would allow the driver to return the appropriate type, so that a php-based driver would return an object, and the xmlrpc-epi driver would return a native php value that was settype()d. The ->retrieveType() example is probably what you might consider to be procedural because it uses a method to retrieve the actual value used, but it is really better OO encapsulation in disguise. By hiding the implementation from the user, you provide a *much* better system for internal implementation, without confusing the user at all. In addition, it uses the principal of API interface as the php, xmlrpc-epi, and xmlrpci drivers would simply have the same method, and all three could be swapped out without a single change to the code. Now, to be fair, Chiara_XML_RPC5 does not have any kind of retrieveType() method. This goes with my other point: if we had this argument offlist BEFORE you brought the thing to a vote, perhaps we could have worked out the differences. The spirit of collaboration (which includes debates over implementation like this) lead to better coding solutions, I am convinced of that. Greg