5

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?

Valentin H
  • 7,240
  • 12
  • 61
  • 111
  • 8
    What about factoring it out into a new function? – TartanLlama Oct 26 '15 at 10:05
  • 2
    This is definitely not a good approach in any kind of production code..... – Kevin Oct 26 '15 at 10:07
  • What about storing comparing functions associated with callback functions ? Then you can iterate with a simple loop. – Richard Dally Oct 26 '15 at 10:09
  • While I am personally interested in this kind of questions, they are definitely off-topic for Stackoverflow because style issues tend to generate opinion-based answers. And "good approach" is very vague. Personally, I would not want to maintain such code, because I think it's hard to read and understand. – Christian Hackl Oct 26 '15 at 10:09
  • I am _certain_ there's a very detailed post about this on SO under the C++ tag, but I can't find it... – Lundin Oct 26 '15 at 10:09
  • 7
    `else if` seems cleaner than that `while (false)`. – Jarod42 Oct 26 '15 at 10:10
  • This is too broad. `a`, `z` etc. mean nothing and such any refactoring would be meaningless, because it would ignore original code. – Bartek Banachewicz Oct 26 '15 at 10:25
  • @Lundin: you mean this one? https://stackoverflow.com/questions/4148838/c-nested-ifs-or-gotos. This is C. I was more interested how to replace this with means of C++ – Valentin H Oct 26 '15 at 10:29
  • @LeFlou: The code example is of cause too simple. There could be many nested among the ifs and many related/unrelated. The break/(goto) is simply used as far as the first condition is meat. – Valentin H Oct 26 '15 at 10:37
  • That must have been very old/dark days when goto was the standard for doing this... – Jens Oct 26 '15 at 10:44
  • @Jarod42: Agree, and that's why I'm asking. I always mark such use of loops with a comment. Actually, I don't use this in a new code, but for refactoring gotos. – Valentin H Oct 26 '15 at 10:45
  • @ValentinHeinitz No it was definitely C++, since I remember that there were some very obscure answers with various wrapper classes and exception handling. It received a lot of attention and I think the question had like 70+ upvotes. Maybe it got nuked for being opinion-based or some nonsense. – Lundin Oct 26 '15 at 10:46
  • This kind of code often shows that there is a design issue. It often means that there are basic abstractions missing. – Jens Oct 26 '15 at 10:49
  • @Jens Often but not always. Any function with a lot of error handling will face these problems. – Lundin Oct 26 '15 at 10:54
  • @Lundin Yes, but these are rare. Most of the error handling is done in RAII, and then we have exception for the exceptional cases. There are some cases though, most of them with file I/O or parsing, but I guess there are better ways to it. Maybe something similar to Haskells Either Monad? – Jens Oct 26 '15 at 11:15

5 Answers5

7

As conditions seem unrelated, else if is an option:

if (a) {
    doA();
} else if (b) {
//...
} else if (z) {
    doZ();
}
Jarod42
  • 203,559
  • 14
  • 181
  • 302
5

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.
ypnos
  • 50,202
  • 14
  • 95
  • 141
  • 1
    " Funny enough early returns are also frowned upon by some." I think not in C++, as you have auto-pointers and scope-based locks. I use many returns in methods. – Valentin H Oct 26 '15 at 10:33
  • 1
    "Another solution could be the use of exceptions" - yes! this is a good approach. B. Stroustrup also advocated for use of ecxeptions as a usual instrument, not only for error-handling – Valentin H Oct 26 '15 at 10:34
  • 5
    Using exception to simulate goto is usually considered as bad as using goto. – Jens Oct 26 '15 at 10:46
  • @Jens I think the context matters. But do you have a source for this statement? – ypnos Oct 26 '15 at 10:52
  • 1
    Another approach could be using `your_favorite_associative_container`. Or possibly, since C++ also supports OO, using an object hierarchy with virtual method (smells like possible CRTP with static polymorphism) and just calling that instead of having a bunch of ifs is an option too. I think you could also use a template method instead of object hierarchy. The list of clean options is very vast. But it doesn't contain `do {} while (false);` – Zdeněk Jelínek Oct 26 '15 at 11:35
  • @ypnos You can find discussions on stackoverflow, stackexchange and many other places. The most concrete source is from J. Bloch in Effective Java: "Exceptions are [...] to be used only for exceptional conditions; they should never be used for ordinary control flow". The new Core guidelines published during cppcon say "Use exceptions for error handling only", which I think transports the same meaning. C++ coding standards by Sutter&Alexandrescu has whole chapter on error handling, and stating multiple times that you should use exceptions only for error handling. – Jens Oct 26 '15 at 11:39
  • @ValentinHeinitz Could you add a reference to the quote of Stroustrup advocating using exceptions to implement control flow? – Jens Oct 26 '15 at 11:45
2

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;
}
Lundin
  • 195,001
  • 40
  • 254
  • 396
1

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.

Richard Hodges
  • 68,278
  • 7
  • 90
  • 142
  • 1
    If you refer to the do-while-trick in macros, that has some other reasons: https://stackoverflow.com/questions/154136/do-while-and-if-else-statements-in-c-c-macros – Jens Oct 26 '15 at 11:41
  • @Jens: Thanks for pointing to the similar usage: http://stackoverflow.com/a/24393101/298206 ! – Valentin H Oct 28 '15 at 15:57
  • @ValentinHeinitz I don't think that is a good argument, and given that there are better alternatives (exceptions, simple if-else, optional types ...), I would step away from that abuse. It is unintuitive and makes the code hard to understand. There is no need to do whatever you can, especially since C++ has so many options to shoot yourself in the foot. I would stay away from tricky solutions as much as possible, and prefer simple and clean alternatives. – Jens Oct 28 '15 at 16:54
-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.

Abhineet
  • 5,320
  • 1
  • 25
  • 43
  • 2
    The fact that Linux code has `goto`s doesn't mean they're any less bad. Neither that, nor your exceptions suggestion make sense without knowing more about the question, though. – Bartek Banachewicz Oct 26 '15 at 10:30
  • 1
    @BartekBanachewicz:: That's why my advise of using `goto` is full of conditions. And I advised using `exceptions` if using C++. – Abhineet Oct 26 '15 at 10:34
  • 2
    I am not against your "conditions". I'm against your [Appeal-To-Authority](https://en.wikipedia.org/wiki/Argument_from_authority) fallacy. – Bartek Banachewicz Oct 26 '15 at 10:38
  • 1
    @BartekBanachewicz:: Got your point now. Very well, yeah the Linux-statement might be considered as formal fallacy. – Abhineet Oct 26 '15 at 10:42
  • 2
    Giving an example of a big, well-maintained software project that uses a specific practice seemingly to success is not necessarily "Appeal-To-Authority". – ypnos Oct 26 '15 at 10:42
  • 1
    There is at least one valid scenario for goto in C (not in C++) which is central error handling/resource handling in functions. In C++, you have much better tools to do this (mainly RAII, exceptions), so I don't really see a good reason to use goto. – Jens Oct 26 '15 at 10:48
  • 1
    The Linux kernel contains lots of really horrible code, almost to the point where it can be used as a proof of _bad_ style: the Linux kernel uses x, so therefore x must be bad :) – Lundin Oct 26 '15 at 10:49
  • @ypnos Correlation is not causation; Linux is successful (to a certain extent) and uses goto, but that doesn't necessarily mean that Linux is successful *because* its codebase has gotos. – Bartek Banachewicz Oct 26 '15 at 11:06
  • And @Abhineet, I still consider breaking out multiple loops - abuse, but perhaps not of `goto` per se. Why do you have nested loops outside of a well-defined function in the first place? – Bartek Banachewicz Oct 26 '15 at 11:07
  • @BartekBanachewicz I'm not saying that there is a causation, or even that I would agree with Abhineet. I never use `goto` myself. I'm just saying that his point of Linux using `goto` is not invalid per-se. It is well-documented and discussed that Linux uses `goto`. So there is more to that than just "Linux uses x so it must be good/bad". – ypnos Oct 26 '15 at 11:39
  • The longstanding hysteria around goto statements is completely overdone. formal loops are nothing more than syntactic sugar, essentially built on top of goto (i.e. a branch instruction). In the days of C it might have been reasonable to argue that the use of goto makes resource deallocation harder to track, but today with RAII this is not so, – Richard Hodges Oct 26 '15 at 11:58
  • In words of Scott Robert Ladd, "A "goto" is not, in and of itself, dangerous -- it is a language feature, one that directly translates to the jump instructions implemented in machine code. Like pointers, operator overloading, and a host of other "perceived" evils in programming, "goto" is widely hated by those who've been bitten by poor programming. Bad code is the product of bad programmers; in my experience, a poor programmer will write a poor program, regardless of the availability of "goto."" – Abhineet Oct 26 '15 at 12:01
  • 1
    There are words in English which are considered bad to say or use, but they all have their significance and usage at proper times. – Abhineet Oct 26 '15 at 12:04
  • I've upvoted your answer, as it is reasonable for me. I thought, nowadays holly-wars are over, but for some people looks like not. – Valentin H Oct 28 '15 at 15:50
  • @ValentinHeinitz: Didn't get it though :-) – Abhineet Oct 29 '15 at 03:35