How to avoid use of goto and break nested loops ef

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 2 years ago.

I'd say that it's a fact that using goto is considered a bad practice when it comes to programming in C/C++.

However, given the following code

for (i = 0; i < N; ++i) 
{
    for (j = 0; j < N; j++) 
    {
        for (k = 0; k < N; ++k) 
        {
            ...
            if (condition)
                goto out;
            ...
        }
    }
}
out:
    ...

I wonder how to achieve the same behavior efficiently not using goto. What i mean is that we could do something like checking condition at the end of every loop, for example, but AFAIK goto will generate just one assembly instruction which will be a jmp. So this is the most efficient way of doing this I can think of.

Is there any other that is considered a good practice? Am I wrong when I say it is considered a bad practice to use goto? If I am, would this be one of those cases where it's good to use it?

Thank you

Answer 1

Alternative - 1

You can do something like follows:

1. Set a bool variable in the beginning isOkay = true
2. All of your forloop conditions, add an extra condition isOkay == true
3. When your your custom condition is satisfied/ fails, set isOkay = false.

This will make your loops stop. An extra bool variable would be sometimes handy though.

bool isOkay = true;
for (int i = 0; isOkay && i < N; ++i)
{
    for (int j = 0; isOkay && j < N; j++)
    {
        for (int k = 0; isOkay && k < N; ++k)
        {
            // some code
            if (/*your condition*/)
                 isOkay = false;
        }
     }
}

---

Alternative - 2

Secondly. if the above loop iterations are in a function, best choice is to return result, when ever the custom condition is satisfied.

bool loop_fun(/* pass the array and other arguments */)
{
    for (int i = 0; i < N ; ++i)
    {
        for (int j = 0; j < N ; j++)
        {
            for (int k = 0; k < N ; ++k)
            {
                // some code
                if (/* your condition*/)
                    return false;
            }
        }
    }
    return true;
}

Answer 2

bool meetCondition = false;
for (i = 0; i < N && !meetCondition; ++i) 
{
    for (j = 0; j < N && !meetCondition; j++) 
    {
        for (k = 0; k < N && !meetCondition; ++k) 
        {
            ...
            if (condition)
                meetCondition = true;
            ...
        }
    }
}

Answer 3

as far as your comment on efficiency compiling the both options in release mode on visual studio 2017 produces the exact same assembly.

for (int i = 0; i < 5; ++i)
{
    for (int j = 0; j < 5; ++j)
    {
        for (int k = 0; k < 5; ++k)
        {
            if (i == 1 && j == 2 && k == 3) {
                goto end;

            }
        }
    }
}
end:;

and with a flag.

bool done = false;
for (int i = 0; i < 5; ++i)
{
    for (int j = 0; j < 5; ++j)
    {
        for (int k = 0; k < 5; ++k)
        {
            if (i == 1 && j == 2 && k == 3) {
                done = true;
                break;
            }
        }
        if (done) break;
    }
    if (done) break;
}

both produce..

xor         edx,edx  
xor         ecx,ecx  
xor         eax,eax  
cmp         edx,1  
jne         main+15h (0C11015h)  
cmp         ecx,2  
jne         main+15h (0C11015h)  
cmp         eax,3  
je          main+27h (0C11027h)  
inc         eax  
cmp         eax,5  
jl          main+6h (0C11006h)  
inc         ecx  
cmp         ecx,5  
jl          main+4h (0C11004h)  
inc         edx  
cmp         edx,5  
jl          main+2h (0C11002h)    

so there is no gain. Another option if your using a modern c++ compiler is to wrap it in a lambda.

[](){
for (int i = 0; i < 5; ++i)
{
    for (int j = 0; j < 5; ++j)
    {
        for (int k = 0; k < 5; ++k)
        {
            if (i == 1 && j == 2 && k == 3) {
                return;
            }
        }

    }
}
}();

again this produces the exact same assembly. Personally I think using goto in your example is perfectly acceptable. It is clear what is happening to anyone else, and makes for more concise code. Arguably the lambda is equally as concise.

Answer 4

If you have deeply-nested loops like that and you must break out, I believe that goto is the best solution. Some languages (not C) have a break(N) statement that will break out of more than one loop. The reason C doesn't have it is that it's even worse than a goto: you have to count the nested loops to figure out what it does, and it's vulnerable to someone coming along later and adding or removing a level of nesting, without noticing that the break count needs to be adjusted.

Yes, gotos are generally frowned upon. Using a goto here is not a good solution; it's merely the least of several evils.

In most cases, the reason you have to break out of a deeply-nested loop is because you're searching for something, and you've found it. In that case (and as several other comments and answers have suggested), I prefer to move the nested loop to its own function. In that case, a return out of the inner loop accomplishes your task very cleanly.

(There are those who say that functions must always return at the end, not from the middle. Those people would say that the easy break-it-out-to-a-function solution is therefore invalid, and they'd force the use of the same awkward break-out-of-the-inner-loop technique(s) even when the search was split off to its own function. Personally, I believe those people are wrong, but your mileage may vary.)

If you insist on not using a goto, and if you insist on not using a separate function with an early return, then yes, you can do something like maintaining extra Boolean control variables and testing them redundantly in the control condition of each nested loop, but that's just a nuisance and a mess. (It's one of the greater evils that I was saying using a simple goto is lesser than.)

Answer 5

In this case you don't wan't to avoid using goto.

In general the use of goto should be avoided, however there are exceptions to this rule, and your case is a good example of one of them.

Let's look at the alternatives:

for (i = 0; i < N; ++i) {
    for (j = 0; j < N; j++) {
        for (k = 0; k < N; ++k) {
            ...
            if (condition)
                break;
            ...
        }
        if (condition)
            break;
    }
    if (condition)
        break;
}

Or:

int flag = 0
for (i = 0; (i < N) && !flag; ++i) {
    for (j = 0; (j < N) && !flag; j++) {
        for (k = 0; (k < N) && !flag; ++k) {
            ...
            if (condition) {
                flag = 1
                break;
            ...
        }
    }
}

Neither of these is as concise or as readable as the goto version.

Using a goto is considered acceptable in cases where you're only jumping ahead (not backward) and doing so makes your code more readable and understandable.

If on the other hand you use goto to jump in both directions, or to jump into a scope which could potentially bypass variable initialization, that would be bad.

Here's a bad example of goto:

int x;
scanf("%d", &x);
if (x==4) goto bad_jump;

{
    int y=9;

// jumping here skips the initialization of y
bad_jump:

    printf("y=%d\n", y);
}

A C++ compiler will throw an error here because the goto jumps over the initialization of y. C compilers however will compile this, and the above code will invoke undefined behavior when attempting to print y which will be uninitialized if the goto occurs.

Another example of proper use of goto is in error handling:

void f()
{
    char *p1 = malloc(10);
    if (!p1) {
        goto end1;
    }
    char *p2 = malloc(10);
    if (!p2) {
        goto end2;
    }
    char *p3 = malloc(10);
    if (!p3) {
        goto end3;
    }
    // do something with p1, p2, and p3

end3:
    free(p3);
end2:
    free(p2);
end1:
    free(p1);
}

This performs all of the cleanup at the end of the function. Compare this to the alternative:

void f()
{
    char *p1 = malloc(10);
    if (!p1) {
        return;
    }
    char *p2 = malloc(10);
    if (!p2) {
        free(p1);
        return;
    }
    char *p3 = malloc(10);
    if (!p3) {
        free(p2);
        free(p1);
        return;
    }
    // do something with p1, p2, and p3

    free(p3);
    free(p2);
    free(p1);
}

Where the cleanup is done in multiple places. If you later add more resources that need to be cleaned up, you have to remember to add the cleanup in all of these places plus the cleanup of any resources that were obtained earlier.

The above example is more relevant to C than C++ since in the latter case you can use classes with proper destructors and smart pointers to avoid manual cleanup.

Answer 6

Break your for loops out into functions. It makes things significantly easier to understand because now you can see what each loop is actually doing.

bool doHerpDerp() {
    for (i = 0; i < N; ++i) 
    {
        if (!doDerp())
            return false;
    }
    return true;
}

bool doDerp() {
    for (int i=0; i<X; ++i) {
        if (!doHerp())
            return false;
    }
    return true;
}

bool doHerp() {
    if (shouldSkip)
        return false;
    return true;
}