| Message ID | 1406860365-5516-2-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
On 07/31/2014 08:32 PM, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > 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 <arei.gonglei@huawei.com> > --- > 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. =====
Hi, > Subject: Re: [PATCH v2 1/8] CODING_STYLE: Section about conditional > statement > > On 07/31/2014 08:32 PM, arei.gonglei@huawei.com wrote: > > From: Gonglei <arei.gonglei@huawei.com> > > > > Yoda conidtions lack of readability, and QEMU have a > > s/conidtions/conditions/ > s/of // > s/have/has/ > OK. > > strict compiler configuration for checking a common > > mistake like "if (dev = NULL)". Make it a written rule. > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > --- > > 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/ > OK. > > + > > +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. > ===== > Good description. > 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. > ===== > Agreed. Thanks! Best regards, -Gonglei
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 + +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.