[Freeipa-devel] [PATCH] jderose 053 XML-RPC signature change

[John Dennis]

On 03/26/2010 05:18 PM, Jason Gerard DeRose wrote:
> On Fri, 2010-03-26 at 09:22 -0400, John Dennis wrote:
>> On 03/26/2010 07:24 AM, Jason Gerard DeRose wrote:
>>> This quick patch changes the XML-RPC signature to match the
>>> complementary change being made in certmonger.
>>>
>>> The signature is now:
>>>
>>>       [args, options]
>>>
>>> This doesn't yet include the [args, options, extra] change... that is
>>> coming in my rpcserver patch once it's done.  But this provides what
>>> needed for current IPA<=>   certmonger compatibility.
>>
>> NAK
>>
>> Is there a reason for the type inconsistency? Why is it a list in one
>> case and a tuple in the other? I realize they'll both operate the same
>> way but the inconsistency is confusing especially if there is no reason
>> to use different type objects (e.g. no need for a mutable sequence).
>
> We use lists and tuples interchangeability.  Tuple are nice because they
> aren't mutable and are a bit more efficient in terms of memory use, but
> both json.loads() and xmlrpclib.loads() return lists.  My general plan
> has been to move to using just lists.

So if the plan is to converge on just using lists then what better time 
to start than now?

>
> json.dumps() and xmlrpclib.dumps() also treat tuples and lists the
> same... both are serialized to a list type.
>
> So there's no type change of any consequence in this patch.

As I said, I know they are interchangeable in most cases. When I read 
code and see differing usage I presume there is a reason and I go 
looking for the reason so that I can follow the correct usage model. If 
there is no actual difference then I've wasted time trying to understand 
why things are different. Consistency has value especially in a huge 
body of code contributed to by many developers.

I realize in this particular case I'm nit-picking and I fully understand 
the API's of xmlrpclib and json but I'm going to push for things like 
consistency, removal of dead code and no code duplication whenever I see 
it. All those things just make it harder to understand the code. It's 
part of the reason we do patch reviews along with finding logic errors.


-- 
John Dennis <jdennis at redhat.com>

Looking to carve out IT costs?
www.redhat.com/carveoutcosts/