Better way to jump in and out a loop without goto?

Question

do {
    int j, k;
    if (num_copy_array[0][0] == num_copy_array[0][1]) {
        goto next_num;
    } else {
        goto first_time;
    }
    for (j = 0; j < ARRAY_LEN; j++) {
        for (k = 0; k < NUM_LEN; k++) {
            goto skip;
        first_time:
            j = 0, k = 2;
        skip:
            for (int j_2 = 0; j_2 <= j; j_2++) {
                for (int k_2 = 0; k_2 <= NUM_LEN; k_2++) {
                    if (j_2 == j && k_2 == k) {
                        goto next_digit;
                    }
                    if (num_copy_array[j][k] == num_copy_array[j_2][k_2]) {
                        goto next_num;
                    }
                }
            }
        next_digit:;
        }
    }
} while (false);
/*
 * some
 * code
*/
:next_num // starts the bigger loop outside

This is how I ended up while writing a c++ program to make a special loop entrance and to break out multiple loops. I had to declare the loop variable 'j, k' outside to have them declared before having jumped into the loop with goto, and used a fake do-loop to localize these.

So-many people say 'never use goto', and I do agree using unnecessary gotos will produce a not good program. However in this case, it is really hard for me to think of an alternative than using temporary switch variables and checking them on every loop entrance, which way I would think as very inefficient and harder to read.

Could you think of a nice way to make this part of my program better, or do you have a general suggestion?

Answer 1

In Elements of Programming, Stepanov and McJones present a state machine that is elegantly coded, even with gotos. And they mention that the obvious non-goto version isn't nearly as elegant. Knuth spends a chapter of Literate Programming to argue for the occasional use of goto. I'm no fan of goto, but I accept it has its uses. However, I always try a non-goto version of any code.

In this case, I think you're trying to simulate named loops, but I don't think you need to. For one, the outer do ... while loop looks an awful lot like a crippled function, that can only be called when some variables are in the right state. You can (and, I argue, should) replace it with a function; and when you do, you'll notice that you no longer need goto skip or first_time.

I started trying to refactor the code, but without knowing what you actually want to do, I'm not able to do much.

Answer 2

Short anwer

Break your code up into functions. You can replace these gotos with judicious use of helper methods and quick returns. Helper methods are a great way to manage the complexity of quadruply nested loops and snaking gotos. They will also reveal the underlying logic of your algorithm by giving names to the various moving parts.

Long answer:

As @MooingDuck deciphered in his comments, the only external effect of this code is to either exit the loop normally or exit it by jumping to the next_num label.

With that in mind, the first step would be to move the loop into a function that returns a true or false controlling whether or not to execute "some code".

if (should_execute_some_code()) {
    /*
     * some
     * code
     */
}

// starts the bigger loop outside

Inside the function, we'll convert any goto next_num line into return true, and otherwise return false if control falls off the bottom of the function.

bool should_execute_some_code()
{
    int j, k;
    if (num_copy_array[0][0] == num_copy_array[0][1]) {
        return true;
    }

    goto first_time;

    for (j = 0; j < ARRAY_LEN; j++) {
        for (k = 0; k < NUM_LEN; k++) {
            goto skip;
        first_time:
            j = 0, k = 2;
        skip:
            for (int j_2 = 0; j_2 <= j; j_2++) {
                for (int k_2 = 0; k_2 <= NUM_LEN; k_2++) {
                    if (j_2 == j && k_2 == k) {
                        goto next_digit;
                    }
                    if (num_copy_array[j][k] == num_copy_array[j_2][k_2]) {
                        return true;
                    }
                }
            }
        next_digit:;
        }
    }

    return false;
}

That's a start. Now how can we get rid of the rest of the gotos? Let's tackle that first_time label. There's no need to jump inside the loop unconditionally. It's not pretty, but we can replace the jump with a conditional initialization for k. Set it to 2 on the first iteration and 0 thereafter. That lets us get rid of the first_time and skip labels.

bool should_execute_some_code()
{
    int j = 0, k = 2;

    if (num_copy_array[0][0] == num_copy_array[0][1]) {
        return true;
    }

    for (int j = 0; j < ARRAY_LEN; j++) {
        for (int k = (j == 0 ? 2 : 0); k < NUM_LEN; k++) {
            for (int j_2 = 0; j_2 <= j; j_2++) {
                for (int k_2 = 0; k_2 <= NUM_LEN; k_2++) {
                    if (j_2 == j && k_2 == k) {
                        goto next_digit;
                    }
                    if (num_copy_array[j][k] == num_copy_array[j_2][k_2]) {
                        return true;
                    }
                }
            }
        next_digit:;
        }
    }

    return false;
}

Next there's the next_digit label. If j == j_2 and k == k_2 you want to stop the inner two loops. That could be controlled with a more selective loop condition instead of a goto.

bool should_execute_some_code()
{
    if (num_copy_array[0][0] == num_copy_array[0][1]) {
        return true;
    }

    for (int j = 0; j < ARRAY_LEN; j++) {
        for (int k = (j == 0 ? 2 : 0); k < NUM_LEN; k++) {
            for (int j_2 = 0; j_2 <= j; j_2++) {
                for (int k_2 = 0; k_2 <= NUM_LEN && !(j_2 == j && k_2 == k); k_2++) {
                    if (num_copy_array[j][k] == num_copy_array[j_2][k_2]) {
                        return true;
                    }
                }
            }
        }
    }

    return false;
}

At this point, it looks to me like you're checking if any entry in the two-dimensional array equals any other entry in the two-dimensional array. And you're careful to ignore checking the same cell against itself. Okay then. Let's rename the function now that we know what it does. Let's also break apart the four nested loops by introducing a second helper function.

bool has_duplicate_entry()
{
    if (num_copy_array[0][0] == num_copy_array[0][1]) {
        return true;
    }

    for (int j = 0; j < ARRAY_LEN; j++) {
        for (int k = (j == 0 ? 2 : 0); k < NUM_LEN; k++) {
            if (contains_entry(num_copy_array[j][k], j, k)) {
                return true;
            }
        }
    }

    return false;
}

bool contains_entry(int entry, int not_j, int not_k)
{
    for (int j = 0; j <= not_j; j++) {
        for (int k = 0; k <= NUM_LEN && !(j == not_j && k == not_k); k++) {
            if (num_copy_array[j][k] == entry) {
                return true;
            }
        }
    }

    return false;
}

Answer 3

You could add another check to your loops, such as:

for (i=0; i < x && !ready; i++)
{
    for (j = 0; j < y && !ready; j++)
    {
        // Now you can maintain the ready variable to finish...
        if (whatever)
            ready = true; // Or, ready = NULL Or ready > 0 etc (whatever suits your situation) 
    }
}

If you can break down your code into more functions so not requiring so many nested loops that may also help.

Answer 4

I suggest something like (with better names):

bool bar(int j, int k)
{
    for(int j_2 = 0; j_2 <= j; j_2++) {
        for(int k_2 = 0; k_2 <= NUM_LEN; k_2++) {
            if(j_2 == j && k_2 == k) {
                return true;
            }
            if(num_copy_array[j][k] == num_copy_array[j_2][k_2]) {
                return false;
            }
        }
    }
    return true;
}

bool foo()
{
    if(num_copy_array[0][0] == num_copy_array[0][1]) {
        return false;
    }
    for(int j = 0; j < ARRAY_LEN; j++) {
        for(int k = (j == 0) ? 2 : 0; k < NUM_LEN; k++) {
            if (bar(j, k) == false) {
                return false;
            }
        }
    }
    return true;
}

void foobar()
{
    if (foo()) {
    /*
     * some
     * code
    */
    }
    // Other code
}