Re: [Qemu-devel] [PATCH v4 16/16] qapi: Prefer '"str" + var' over '"str%s" % var'

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1660 bytes --]

On 05/14/2015 10:09 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Use of python's % operator to format strings is fine if there are
>> multiple strings or if there is integer formatting going on, but
>> when it is just abused for string concatenation, it looks nicer
>> to just use the + operator.  This is particularly true when the
>> value being substituted is at the front of the format string,
>> rather than the tail.
> 
> I quite agree in cases such as 
> 
> -        discriminator_type_name = '%sKind' % (name)
> +        discriminator_type_name = name + 'Kind'

I could always split this into the obvious cases vs. the questionable
ones, if that helps.

> 
> I have doubts in cases such as
> 
> -    return "qmp_marshal_output_%s(retval, ret, &local_err);" % c_name(name)
> +    return "qmp_marshal_output_" + c_name(name) + "(retval, ret, &local_err);"
> 
> I find the old code makes it easier to grasp the result.  Admittedly
> subjective.

Yeah, that's a judgment call.

> 
>> Update an error message (and testsuite coverage) while at it, since
>> concatenating a non-string object does not always produce legible
>> results.
> 
> The new expected test output shows improvement.

Also might be worth splitting into its own patch, rather than buried in
the noise of cleanups.

> 
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> I'll take 01-15 now, and have a second look at this one later, okay?

Yeah, I kind of figured that might happen. Works for me :)

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]