qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like
Date: Tue, 08 Mar 2016 11:54:23 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

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.

> Suggested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>

Patch looks good.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]