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

[libvirt] [PATCH] maint: document dislike of mismatched if/else bracing



* docs/hacking.html.in (Curly braces): Tighten recommendations to
disallow if (cond) one-line; else { block; }.
* HACKING: Regenerate.
Suggested by Daniel P. Berrange.
---

> > since HACKING documents that an else clause should only ever omit braces
> > when the if clause also omitted braces, but an if clause can omit braces
> > even when the else clause requires them.
> Hmm, I didn't notice that. I really don't like to see braces in
> else clauses, without also seeing braces in the if, and have
> been fixing this to add braces whenever I come across it.

Fine by me - how does this look?

 HACKING              |   35 ++++++++++++++++++++++-------------
 docs/hacking.html.in |   34 +++++++++++++++++++++++-----------
 2 files changed, 45 insertions(+), 24 deletions(-)

diff --git a/HACKING b/HACKING
index d646709..4a71b37 100644
--- a/HACKING
+++ b/HACKING
@@ -161,33 +161,42 @@ Do this, instead:

 However, there is one exception in the other direction, when even a one-line
 block should have braces. That occurs when that one-line, brace-less block is
-an "else" block, and the corresponding "then" block *does* use braces. In that
-case, either put braces around the "else" block, or negate the "if"-condition
-and swap the bodies, putting the one-line block first and making the longer,
-multi-line block be the "else" block.
+an "if" or "else" block, and the counterpart block *does* use braces. In that
+case, put braces around both blocks. Also, if the "else" block is much shorter
+than the "if" block, consider negating the "if"-condition and swapping the
+bodies, putting the short block first and making the longer, multi-line block
+be the "else" block.

   if (expr) {
       ...
       ...
   }
   else
-      x = y;    // BAD: braceless "else" with braced "then"
+      x = y;    // BAD: braceless "else" with braced "then",
+                // and short block last

-This is preferred, especially when the multi-line body is more than a few
-lines long, because it is easier to read and grasp the semantics of an
-if-then-else block when the simpler block occurs first, rather than after the
-more involved block:
+  if (expr)
+      x = y;    // BAD: braceless "if" with braced "else"
+  else {
+      ...
+      ...
+  }

-  if (!expr)
+Keeping braces consistent and putting the short block first is preferred,
+especially when the multi-line body is more than a few lines long, because it
+is easier to read and grasp the semantics of an if-then-else block when the
+simpler block occurs first, rather than after the more involved block:
+
+  if (!expr) {
     x = y; // putting the smaller block first is more readable
-  else {
+  } else {
       ...
       ...
   }

-If you'd rather not negate the condition, then at least add braces:
+But if negating a complex condition is too ugly, then at least add braces:

-  if (expr) {
+  if (complex expr not worth negating) {
       ...
       ...
   } else {
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 900e242..0d81b0b 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -209,11 +209,13 @@
     <p>
       However, there is one exception in the other direction, when even a
       one-line block should have braces.  That occurs when that one-line,
-      brace-less block is an <code>else</code> block, and the corresponding
-      <code>then</code> block <b>does</b> use braces.  In that case, either
-      put braces around the <code>else</code> block, or negate the
-      <code>if</code>-condition and swap the bodies, putting the
-      one-line block first and making the longer, multi-line block be the
+      brace-less block is an <code>if</code> or <code>else</code>
+      block, and the counterpart block <b>does</b> use braces.  In
+      that case, put braces around both blocks.  Also, if
+      the <code>else</code> block is much shorter than
+      the <code>if</code> block, consider negating the
+      <code>if</code>-condition and swapping the bodies, putting the
+      short block first and making the longer, multi-line block be the
       <code>else</code> block.
     </p>

@@ -223,31 +225,41 @@
       ...
   }
   else
-      x = y;    // BAD: braceless "else" with braced "then"
+      x = y;    // BAD: braceless "else" with braced "then",
+                // and short block last
+
+  if (expr)
+      x = y;    // BAD: braceless "if" with braced "else"
+  else {
+      ...
+      ...
+  }
 </pre>

     <p>
-      This is preferred, especially when the multi-line body is more than a
+      Keeping braces consistent and putting the short block first is
+      preferred, especially when the multi-line body is more than a
       few lines long, because it is easier to read and grasp the semantics of
       an if-then-else block when the simpler block occurs first, rather than
       after the more involved block:
     </p>

 <pre>
-  if (!expr)
+  if (!expr) {
     x = y; // putting the smaller block first is more readable
-  else {
+  } else {
       ...
       ...
   }
 </pre>

     <p>
-      If you'd rather not negate the condition, then at least add braces:
+      But if negating a complex condition is too ugly, then at least
+      add braces:
     </p>

 <pre>
-  if (expr) {
+  if (complex expr not worth negating) {
       ...
       ...
   } else {
-- 
1.7.3.4


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