[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/8] CODING_STYLE: Section about conditional
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/8] CODING_STYLE: Section about conditional statement |
Date: |
Thu, 31 Jul 2014 21:07:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.7.0 |
On 07/31/2014 08:32 PM, address@hidden wrote:
> From: Gonglei <address@hidden>
>
> Yoda conidtions lack of readability, and QEMU have a
s/conidtions/conditions/
s/of //
s/have/has/
> strict compiler configuration for checking a common
> mistake like "if (dev = NULL)". Make it a written rule.
>
> Signed-off-by: Gonglei <address@hidden>
> ---
> CODING_STYLE | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/CODING_STYLE b/CODING_STYLE
> index 4280945..11a79d4 100644
> --- a/CODING_STYLE
> +++ b/CODING_STYLE
> @@ -91,3 +91,22 @@ Mixed declarations (interleaving statements and
> declarations within blocks)
> are not allowed; declarations should be at the beginning of blocks. In other
> words, the code should not generate warnings if using GCC's
> -Wdeclaration-after-statement option.
> +
> +6. Conditional statement
s/statement/statements/
> +
> +Please don't use Yoda conditions because of lack of readability. Furthermore,
> +it is not the QEMU idiomatic coding style. Example:
> +
> +Usually a conditional statement in QEMU would be written as:
> +if (a == 0) {
> + /* Reads like: "If a is equal to 0..." */
> + do_something();
> +}
> +
> +Yoda conditions describe the same expression, but reversed:
> +if (0 == a) {
> + /* Reads like: "If 0 equals to a" */
> + do_something();
> +}
> +
> +The constant is listed first, then the variable being compared to.
>
This spends more lines documenting the bad style than the good, and
doesn't quite flow with the rest of the document. At the risk of
sounding like a complete rewrite, how about:
=====
When comparing a variable for (in)equality with a constant, list the
constant on the right, as in:
if (a == 0) {
do_something();
}
Rationale: Yoda conditionals (as in 'if (0 == a)') are awkward to read.
Besides, good compilers already warn users when == is mis-typed as =,
even when the constant is on the right.
=====
and maybe some other ideas also worth adding:
=====
Avoid redundant comparisons: (bool_expr == true) is better written as
(bool_expr), and (ptr == NULL) is shorter as (!ptr). Use of !!value is
a convenient shorthand for converting a value into a boolean.
=====
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 for-2.2 0/8] about Yoda conditions, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 8/8] vmxnet3: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 3/8] audio: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 2/8] usb: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 1/8] CODING_STYLE: Section about conditional statement, arei.gonglei, 2014/07/31
- Re: [Qemu-devel] [PATCH v2 1/8] CODING_STYLE: Section about conditional statement,
Eric Blake <=
- [Qemu-devel] [PATCH v2 6/8] spice: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 7/8] vl: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 5/8] a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- [Qemu-devel] [PATCH v2 4/8] isa-bus: a trivial code change for more idiomatic writing style, arei.gonglei, 2014/07/31
- Re: [Qemu-devel] [PATCH v2 for-2.2 0/8] about Yoda conditions, Eric Blake, 2014/07/31