C - do{..} while(0); can be removed from code excluding usage nested if else?

Question

do{...} while(0);

the usage of do{}while(0); in my coding is used because, i do not want to use long if else nested conditional statements. I eventually give an break at the time of failure and move out of the loop, with a assurance that my function would have been traversed at least 1 time.

Now, the problem comes with the code warning tools, I am getting a warning at the usage of do{...}while(0);

the usage of nested if(){} else{} is less readable, high complex. and lets the code to be having dead code.

if i exclude nested if(){} else{} and do{} while(0); , do we left part with some other way to make code readable with understandable logic;

if(status_of_funcA_ok != funcA())
{ //failure}
else if (status_of_funcB_ok != funcB())
{//failure}
else if (status_of_funcC_ok != funcC())
else
{//Great}

do{
   if(status_of_funcA_ok != funcA())
   break;
   if (status_of_funcB_ok != funcB())
   break;
   if (status_of_funcC_ok != funcC())
   break;

  }while(0);

http://stackoverflow.com/questions/10720465/do-while0-vs-if-1-in-macros has not explained much to the problem, and has now got close. :( — Ashutosh, Jul 11 '12 at 05:08
could u give a concrete example of a part of your code plz? maybe it is possible to rewrite it differently — AndersK, Jul 11 '12 at 05:13
Compilers can warn about anything they like; it's impractical to avoid all warnings in all cases. *Most* warnings are important, but if you know what you're doing (i.e., you actually know better than the compiler in this case), you can ignore them. — Keith Thompson, Jul 11 '12 at 05:32

score 5 · Answer 1 · answered Jul 11 '12 at 06:24

Move the complete logic of the do while{0} loop to a function, and replace the break with return. And call the function, instead of the loop.

You will not have to worry about the beauty.
The compiler also doesn't have to complain about the do while{0}.

All the more, by adding a bit of modularity, the program might be a little more readable.

In any case, before doing this, it would be nice to check whether your compiler is in an extremely pedantic mode, and you might want to turn that off. That might take the warning away.

ss.

PS: You don't seem to need a return value for the function, but you could have that to get a clue of which function was successful.

peterchen · Answer 2 · 2012-07-11T13:17:49.057

I am using this pattern too, for those who wonder, here's an abstract example:

do   // while(0) for break
{ 
  state1 = 0;
  if (cond1())
  {
    if (cond2())
      break;
    state1 = opA();
  }

  if (cond3() || state1 && state1->cond4())
    break;
  ... 
  Triumph(state1, ...);
  // often here: return
}
Failure(state1, ...);

I consider this valid in the following circumstances:

you have a long-ish sequence (say, >~half a dozen of conditions)
the conditions are complex, and you use / build up significant state, so you can't isolate the elements into functions
you are in an exception-unfriendly environment, or your break-ing code path is not actually an exception

What you can do about it:

Silence the warning. It is just a warning, after all; ~~and I don't see a "typical mistake" (like typing 0 instead of your condition) that would be caught by this warning.~~

[edit] Now, that was silly. the typical mistake that you catch with the warning is e.g. while (a1!=a1) instead of while (a1!=a2).[/edit]

Break into functions, move state to a class
this would transform above code to:

struct Garbler
{
  State1 state1;

  bool Step1()
  {
     state1 = 0;
     if (cond1())
     {
        if (cond2())
          return false;
        state1 = opA();
     }
     return true;
  }

  bool Step2()
  {
     return cond3() || state1 && state1->cond4();
  }
  .. 

  void Run()
  {
    if (Step1() && Step2() && ... && Step23())
      Triumph(state1, ...);
    else
      Failure(state1, ...);
  }
}

This is arguably less readable, worse is that you pull apart the sequence, which might lead to a very questionable class (where members may be called only in a certain order).

Scopeguards
This may allow to transform the breaks into early returns, which are more acceptable:

state1 = 0;
ScopeGuard gFailure = MakeGuard(&Failure, ByRef(state1), ...);
if (cond1())
{
   if (cond2())
     return;
   state1 = opA();
}

if (cond3() || state1 && state1->cond4())
  return;

// everything went ok, we can dismiss the scopeguard
gFailure.Dismiss();
Triumph(state1, ...);

They can be more elegantly written in C++0x, preserve the flow, but the solution isn't that flexible either, e.g. when Failure() cannot be isolated easily into a single function.

A very precise and full answer. My habits are (more-or-less) the same. I have exactly the same habits. — valdo, Jul 11 '12 at 08:27

perreal · Answer 3 · 2012-07-11T06:16:03.190

1

You can use goto instead of do {} while(0) and break. This is not readable and not good practice either though. I think for each specific case there is a better way to avoid deep if/else structures. For example, sometimes using function calls can help:

for example instead of:

if(status_of_funcA_ok != funcA())
{ //failure}
else if (status_of_funcB_ok != funcB())
{//failure}
else if (status_of_funcC_ok != funcC())
else
{//Great}

you can write:

if (check_funcs() == 0) {
   great();
}

int check_funcs() {
   if (status_of_funcA_ok != funcA())
     return -1;
   if (if(status_of_funcB_ok != funcB())) 
     return -2;
   if (if(status_of_funcC_ok != funcC())) 
     return -3;
   return 0; /* great */
}

Sometimes, you can use exit().

Also, in c++ you can use throw() and try/catch:

try {
   /* */
  throw (this error);
   /* */
  throw (that error);
} catch (this error) {
} catch (that error) {
}

edited Jul 11 '12 at 06:16

answered Jul 11 '12 at 05:15

perreal

94,503
21
155
181

5

throw() and catch() should be reserved for exceptional circumstances only (exceptions). it is bad practice to use them as flow control structures. – bancsy Jul 11 '12 at 05:27
thanks, for the reply.. I think goto/return would have been a better option but .. the foolish nasty tool has these things also as an warning. as **multiple break, usage of goto.** as i am using C, throw and catch would not be applicable, i can use sub helper function pointers which will be wrapping up each condition, within a loop i can run these functions. but for small functions of more or less 4 to 5 conditional check, but does this approach acceptable in embedded programming is a serious doubt as i am very new to industry. – Ashutosh Jul 11 '12 at 05:37
1

@bancsy: They are flow control structures *in principle*, how else can you use them if not to control flow? They should used to escape code paths where it's not meaningful to continue, whether or not that can happen a lot depends on your situation. Often you can have a function that checks for the preconditions that will cause your processing function to throw and returns a boolean, so you can opt-out of exceptions, in a way. – GManNickG Jul 11 '12 at 05:52
`goto` ain't good in C++ if objects with c'tor/d'tor are present, raw jumping may bypass them. – valdo Jul 11 '12 at 06:07
you are correct sir, this question is much related to C @valdo – Ashutosh Jul 11 '12 at 06:33
1

@valdo: most newer compiler revisions won't compile code with `goto` that will cause problems with ctors/dtors (as fw as a few other cases). – Necrolis Jul 11 '12 at 06:35
@GManNickG: I guess a more appropriate wording of the second sentence should be: "It is bad practice to use them as replacement for if-else." – bancsy Jul 11 '12 at 07:42
1

@Valdo: [Goto does not leak resources](http://stackoverflow.com/questions/7334952/will-using-goto-leak-variables) – Juraj Blaho Jul 11 '12 at 08:13

score 1 · Answer 4 · answered Jul 11 '12 at 05:39

Nested nested if-else statements can become quite unreadable, but I think using do {..} while(0); as a replacement would be much worse. It is very unconventional and anybody else reading it would not really associate it with if-else statements.

There are a few things you can do to make nested if-else statements more readable. A few suggestions are:

optimize your logic - sometimes you can do away with a lot of if clauses when you 'refactor' your logic ex. grouping identical items.
use switch() - switch is generally more readable compared to if-else statements. You can associate an enum to each case and you can switch this.
encapsulate complicated logic with functions

score 0 · Answer 5 · answered Jul 11 '12 at 05:38

0

If there are more conditions to check avoid using if{} else{},

best practice is to Replace if else conditions with switch case

answered Jul 11 '12 at 05:38

Munipratap

529
5
9

C - do{..} while(0); can be removed from code excluding usage nested if else?

5 Answers5