From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45948) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adFHZ-00070d-JL for qemu-devel@nongnu.org; Tue, 08 Mar 2016 05:54:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1adFHW-00034H-DW for qemu-devel@nongnu.org; Tue, 08 Mar 2016 05:54:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34379) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1adFHW-000348-8B for qemu-devel@nongnu.org; Tue, 08 Mar 2016 05:54:26 -0500 From: Markus Armbruster References: <1457194595-16189-1-git-send-email-eblake@redhat.com> <1457194595-16189-4-git-send-email-eblake@redhat.com> Date: Tue, 08 Mar 2016 11:54:23 +0100 In-Reply-To: <1457194595-16189-4-git-send-email-eblake@redhat.com> (Eric Blake's message of "Sat, 5 Mar 2016 09:16:28 -0700") Message-ID: <87io0xb5pc.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v4 03/10] qapi: Make c_type() more OO-like List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake 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 > Signed-off-by: Eric Blake Patch looks good.