C usage of goto

Question

I have a state machine with some kind of callback mechanism were functions are invoked by a task runner. Behind all of that is a state machine which has 4 kinds of states, some excluding each other, others can be combined, which makes quite a complex rule set. One of the functions should show the user error messages if any illegal action is attempted (for sake of simplicity printf here):

static int state1 = 0;
static bool switch2 = 1;

void do_stuff(int value){
    int errorCode = 0;
    if(state1 == 1){ 
        errorCode = -1;
        goto ERROR;
    }
    if(state1 == 2 && switch2)
    {
        errorCode = 2;
        goto ERROR;
    }


    printf("No error!");
    return;
ERROR:
    printf("%d", errorCode);
}

This was the shortest and most concise way I can think of, but its always been hammered home that using goto is a bad thing. Is there any better way to solve this problem or is this the best way in regards of stability and maintenance?

you may want to take a look at this answer: https://stackoverflow.com/questions/46586/goto-still-considered-harmful — mjl007, Aug 17 '20 at 17:55
Gotos are prefectly acceptable to implement state machines (ducks and runs for cover). But your code is a mix of state variables and gotos, I don't like that. — john, Aug 17 '20 at 17:56
See also https://softwareengineering.stackexchange.com/a/133523/33478 — Keith Thompson, Aug 17 '20 at 17:59
Having a `Finally`/`OnScopeEnd` class would allow to avoid the return — Jarod42, Aug 17 '20 at 17:59
You're not only abusing `goto` here, but all you've done is make a janky version of exceptions. Define your own exception class that includes your `errorCode` value and `throw` that. — tadman, Aug 17 '20 at 18:00
@john They're risky, confusing, and hard to understand at a glance which is why they're best left to machine generated code if and only if they're necessary for performance reasons. — tadman, Aug 17 '20 at 18:02
@Jarod42 can you give an example for that? I do not quite undestand — Curunir, Aug 17 '20 at 18:06
I talk about (C++) RAII, `struct OnScopeEnd{ ~OnScopeEnd() { func(); } std::function func};` and `int error_code= 0; OnScopeEnd onScopeEnd{[&](){if (error_code != 0) printf("%d", error_code)}}; /*..*/`. — Jarod42, Aug 17 '20 at 18:19
It is a sadly pervasive overgeneralization that "using goto is a bad thing". There are always alternatives in C and especially in C++. There are *usually* better alternatives, especially in C++. However, in the gap between "usually" and "always" there are cases where careful, strategic use of `goto` is the cleanest and clearest alternative available. Rejecting the cleanest, clearest code available because of a dogmatic belief that `goto` is bad, bad, bad! is not a rational decision. — John Bollinger, Aug 17 '20 at 18:21
@Curunir Please clarify if you are compiling this code with c or c++. It happens that the shown code will compile in both languages, but many of the suggested solutions in the comments are strictly c++. If you want c++ solutions, then perhaps change `printf` to `cout` to avoid any confusion. — cigien, Aug 17 '20 at 18:27

cigien · Accepted Answer · 2020-08-17T18:05:02.073

8

goto is rarely the right solution for control flow. While there are valid use cases for goto, in this particular function you could simply restructure the control flow into if-else branches like this:

void do_stuff(int value)
{
    int errorCode = 0;

    if (state1 == 1)
    { 
        errorCode = -1;
    }
    else if (state1 == 2 && switch2)
    {
        errorCode = 2;
    }
    else // unconditional case for no errors
    {
        printf("No error!");
        return;
    }
    printf("%s", errorCode);  // if control reaches here, print the error
}

edited Aug 17 '20 at 18:05

answered Aug 17 '20 at 17:57

cigien

57,834
11
73
112

4

He could. But please point out the problem with `goto`. I don't see much of an improvement and I don't see any strong argument against `goto`. – Cheatah Aug 17 '20 at 18:27
@Cheatah My answer simply takes for granted (as does the question) that `goto` is harmful. I'm answering the question, "Is there a better way?" Also, personally, I think not using goto *is* a major improvement. Usually, I feel the onus should be on justifying a usage of `goto` rather than the other way around. – cigien Aug 17 '20 at 18:39
1

@cigien It goes both ways. Sometimes, you see gymnastics, in an atempt to avoid `goto` at all costs, in code where the use of `goto` could have simplified it. So it generally depends on the specific use-case at hand rather than needing to justify one way or other. – P.P Aug 17 '20 at 19:03
@P.P Fair enough. I tend to err on one side more than the other, but I see your point. In this case at least, there seems to be no reason to use goto, and the alternative is perfectly readable. – cigien Aug 17 '20 at 19:11
This solution is convoluted and only looks somehow clean because there is no cleanup code in each error case. – Acorn Aug 17 '20 at 19:16
@Acorn Sure, I'm only showing a solution to the OP's example. If the OP's code was different, then using `goto` might well be cleaner, but that would depend on the particular code. – cigien Aug 17 '20 at 19:20

score 2 · Answer 2 · answered Aug 17 '20 at 19:13

From what I can tell, you are using goto for error handling, not for a state machine.

Using goto for error handling is actually one of those cases where it is very useful and preferred vs. a convoluted chain of conditions. It allows you to perform manual RAII without repetition of code:

int do_stuff(...)
{
    ... = f1(...);
    if (...)
        goto ERROR_f1;

    ... = f2(...);
    if (...)
        goto ERROR_f2;

    ... = f3(...);
    if (...)
        goto ERROR_f3;

    // Success
    return ...;

ERROR_f3:
    undo_f2(...);

ERROR_f2:
    undo_f1(...);

ERROR_f1:
    return ...;
}

C usage of goto

2 Answers2