Re: [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like

[Eric Blake]

On 03/08/2016 03:54 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> QAPISchemaType.c_type() was a bit awkward.  Rather than having two
>> optional orthogonal boolean flags that should never both be true,
>> and where all callers should pass a compile-time constant (well,
>> our use of is_unboxed wasn't constant, but a future patch is about
>> to remove the special case for simple unions, so we can live with
>> the churn of refactoring the code in the meantime), the more
>> object-oriented approach uses different method names that can be
>> overridden as needed, and which convey the intent by the method
>> name.  The caller just makes sure to use the right variant, rather
>> than having to worry about boolean flags.
>>
>> It requires slightly more Python, but is arguably easier to read.
> 
> The second sentence is rather long.  Suggest:
> 
>     QAPISchemaType.c_type() is a bit awkward: it takes two optional
>     boolean flags is_param and is_unboxed that should never both be
>     true.
> 
>     Add a new method for each of the flags, and drop the flags from
>     c_type().
> 
>     Most calls pass no flags.  They remain unchanged.
> 
>     One call passes is_param=True.  Call new .c_param_type() instead.
> 
>     One call passes is_unboxed=True except for simple union types.  This
>     is actually an ugly special case that should go away soon.  Until
>     then, we now have to call either .c_type() or the new
>     .c_unboxed_type().  Tolerable.

Yes, that works for me.

> 
>> Suggested-by: Markus Armbruster <address@hidden>
>> Signed-off-by: Eric Blake <address@hidden>
> 
> Patch looks good.
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature