"Abusing" loops for reducing if-nesting

Question

Sometime one have to implement a series of if/else checks. In old-days goto was the sandard instrument for this.

As goto is a no-go in many code style guides, I sometimes use loops as a replacement e.g.:

do
{
  if (a)
  {
   doA();
   break;
  }
  //...
  if (z)
  {
   doZ();
   break;
  }
}while(false);

//all break jump here

Is it a good approach? Is there a good C++ pattern (e.g. using templates, inheritance, etc.) for implementing this without too much overhead?

Answer 1

IMHO, I don't see any wrong in use of goto. If you know, how, where and why to use goto, your code could be more cleaner and readable.

For example, Linux uses a lot of goto statements. As stated by @Bartek Banachewicz, "The fact that Linux code has gotos doesn't mean they're any less bad."

goto is not always bad, they have many valid uses, like breaking out of a deeply nested loop. Using goto in that case will render a much cleaner code. It leads to spaghetti code only when abused or used where they are not meant to be like in C++ programs.

Note: Unnecessary use of any loop, if-else statement, break, continue and what-not, will lead to messy and unmanageable code. Even if we make this world a un-goto place, spaghetti code will exist. It's upto the individual to make sure he/she writes a cleaner, readable, optimal code.

The best design is to use exceptions in C++ programs.

If you are out of your option to use exceptions, then the other design could be in using another method with multiple cases grouped together in that.

Answer 2

Your non-loop is slightly abusive to your fellow programmers, but not horribly so. You see a lot of code like this in the implementation of assertion macros and loggers.

With templates of course, the opportunities for abuse expand geometrically. For example, you can write this kind of thing:

void doA() {
    std::cout << "A" << std::endl;
}

void doB() {
    std::cout << "B" << std::endl;
}



void either_or() {}

template<class T>
void either_or(T&& t) {
    if (std::get<bool>(t)) {
        std::get<void(*)()>(t)();
    }
}

template<class T, class...Ts>
void either_or(T&& t, Ts&&... ts)
{
    if (std::get<bool>(t)) {
        std::get<void(*)()>(t)();
    }
    else {
        either_or(std::forward<Ts>(ts)...);
    }
}

auto main() -> int
{
    using namespace std;

    bool a = false;
    bool b = true;

    either_or(make_tuple(a, &doA),
              make_tuple(b, &doB));

    return 0;
}

Which while an efficient way (after optimiser intervention) of performing the first action for which the corresponding flag is true, is arguably more abusive to maintainers.

Answer 3

Using goto, using continue, using multiple break, or using multiple return are all, if abused, different flavours of spaghetti coding, all them could be used to create non-conditional branches. Some examples of abuse:

• goto is considered very bad if used for anything else but a jump downwards, for example to an error handler. (While that may be an acceptable use of goto, it still starts the whole beating-the-dead-horse goto-is-considered-harmful debate, so I'd avoid goto for that reason alone.)

• continue non-conditionally jumps upwards, which is considered bad: one definition of spaghetti code is code which jumps non-conditionally both upwards and downwards. It is particularly bad when multiple continue are added to the same loop. In generally, the existance of continue is a pretty certain sign of a loop that needs to be rewritten.

• Multiple break and multiple return can be abused to break out from complex, nested loops, making the code hard to read and maintain. Also, the break method as demonstrated in the question, enforces the use of the somewhat obscure do-while(false) loop.

Generally, all of these methods are considered bad practice since they can easily be abused. You'll simply have to pick the one which is the least bad: in other words the most readable and least obscure.

I believe that multiple returns is the most readable form, since it can be mixed in with some function result variable, which you'll possibly want to have anyway:

result_t func ()
{
  if (a)
  {
    doA();
    return RESULT_A;
  }

  ...  // you'll need lots of if statements here to justify this program design

  if (z)
  {
    doZ();
    return RESULT_Z;
  }

  return RESULT_NORMAL;
}

---

A side note regarding resource allocation inside the function.

If the above function needs to free some allocated resources and it needs to do so always, you should do that with RAII, so that when the local variable goes out of scope, it cleans up itself.

If it only needs to free some allocated resources in some cases (for example upon error), then you should not do that inside every if statement (unmaintainable), nor should you implement some 1980s BASIC programmer's "on error goto". Instead, consider adding a wrapper function outside the main function, to make the program maintainable and minimize the clutter:

result_t wrapper ()
{
  stuff = allocate(); // if needed

  result_t result = func(stuff);

  if(result == BAD)
  {
    deallocate(stuff);
  }

  return result;
}

Answer 4

In general for writing maintainable code you should use the tools of the language in the way they are intended. Your usage of a loop that is supposed to only loop for one iteration is clearly a deflection not intended by the very same set of code style guidelines that frown upon goto in the first place.

There are some tools available in the language that make it easier for you to communicate to other programmers and your further self what you are intending to do here.

• One of them is to outsource this code block into a separate method and use the return statement. Funny enough early returns are also frowned upon by some.
• Another solution could be the use of exceptions, where you try/catch. Whether it fits your purpose mostly depends on the kind of "checks" you are doing.
• Finally, a chain of if/else might not look very elegant but on the other hand is a construct that leaves few room for misinterpretation.

Answer 5

As conditions seem unrelated, else if is an option:

if (a) {
    doA();
} else if (b) {
//...
} else if (z) {
    doZ();
}