Thread: I used my first goto statement today!

Originally Posted by MutantJohn

How does your method affect the call stack though?

It is just one more function call, obviously.

Originally Posted by MutantJohn

The only reason I ask is because I have recursive code that overflows the stack with calls. So today I made it iterative and not recursive.

That is no reason to avoid the appropriate use of functions. If necessary, use an explicit stack.

Well, you need to figure out what data needs to be saved so you can create stacks of the appropriate type(s). So, you have this code that does the stuff in delaunay. When you need to do the stuff that is currently in two_to_three (or when you need to call delaunay in two_to_three), you save the data required for delaunay in its stack. At some point, you will reach what was the base case for the recursion, upon which you pop from the appropriate stack and do whatever processing is left to do, then pop from the other stack, etc.

Originally Posted by anduril462

Hmm...not the worst use of a goto, but probably not the best either. There have been times that I really wish C supported labeled break/continue statements, but that's becoming rare. I am more than happy to simply move stuff into helper functions:

Code:

for (auto it = vec.begin(); it < vec.end(); ) {
    process_vector_elements()
}
...
void process_vector_elements()
{
    for (int i = 0; i < 4; ++i) {  // use pre-increment in C++
        if (t[i]) {
            switch (x) {
                case 23:
                    return;

When it hits that return, and process_vector_elements is done, the vector loop will proceed to the next element.

As for the pre-increment, it avoids having to call a copy constructor (since it doesn't have to save the old value of i).

The other nice thing about splitting up your function like this is you've separated the processing of a vector of objects, from the processing of a single object. That keeps your code loosely coupled and makes it easier to do things like unit test your code, since you have isolated the processing of a single foo object. And it will keep your code (or at least this part of it) goto-free.

Also, I hope you're not using magic numbers (4 and 23) in the real code. Some #defines and enums would do nicely here.

Last note:
This definitely seems to be C++. I'm not much of a C++ expert, but I can't recall ever seeing a good use of goto in C++. In C, however, the most common use that I've seen (and personally the only one I generally think is a good use of goto) is for error handling, so you can avoid deeply-nested if statements or repeating tedious cleanup code.

I don't get it. So instead of using goto in a proper, structured manner, you've resorted to breaking up a loop in a completely unnatural way (nevermind the issue of having clean access to variables from the calling function)? It's really very simple:

Code:

    for( ;; )
        for( ;; )
            goto done;
    done:;

Or even:

Code:

    try
    {
        for( ;; )
            for( ;; )
                throw 1;
    }
    catch( int )
    {    }

Sorry, I just felt like sharing. goto's are actually pretty sweet. I'm going to needlessly fill my code with them from now on.

O_o

I get that this is hyperbole, but you managed your first `goto' in a language with scoped destruction and real world compilers that don't always follow the standard.

Now, I'm not saying this will cause you problems here (The chosen example is fixed construct really so no different than `break'.), but pretty much every compiler I've ever worked on has produced invalid code on complicated `goto' and similar constructs.

To be clear, the standard specifically mandates that destructors be called however execution leaves a block, but with calculated `goto', or functionality that is otherwise problematic to statically analyze, most compilers aren't perfect.

As usual when this comes, my absurdly broad experience with breaking compilers, I am only making you aware of the potential issue you may wish to avoid on the off chance as the situation I reference is despicably hard to debug.

The only reason I ask is because I have recursive code that overflows the stack with calls. So today I made it iterative and not recursive.

Well, you've already made it iterative, and, as I infer from the example, the major iteration is broken over partial iteration.

Your problem is literally already solved; you only need to massage the partial iteration into a consumable form.

You could use a dumb `continue' function, but I'd recommend continuing with the iterator pattern:

Code:

typedef /* whatever */ elem;
// ...
bool yield(elem)
{
    switch(elem)
    {
        /* partial logic */
        case 23: return false;
        // ...
    }
}
// ...
for(/*major*/)
{
    partial<elem> inner(it, vec.end(), yield)
    partial<elem> end;
    // ...
    while(inner < end)  
    {
        // ...
        // use *inner as it[i]
        // ...
    }
}
// ...

Of course, to me nothing is more natural than processing collections iteratively; your mileage may vary.

The idea would be the same without employing the iterator pattern so heavily:

Code:

typedef /* whatever */ elem;
// ...
bool continue(partial & it, partial end, int & counter)
{
    switch(/* *it[counter] */)
    {
        /* partial logic */
        case 23: return false;
        // ...
    }
}
// ...
for(/*major*/)
{
    int counter(0);
    while(continue(it, end, counter))  
    {
        // ...
        if(valid(it))
        {
            // whatever
        }
        else
        {
            // remove from/insert into vec at it [two_to_three(it, end)];
            // or whatever again using iterators from the partial process
        }
        // ...
    }
}
// ...

[Edit]
Yes, I realize `continue' is a keyword; I chose it for the sake the familiar meaning not suggesting this is actual code.
[/Edit]

These are just examples of course, but crucially to the concept, you would not have "resorted to breaking up a loop in a completely unnatural way"; the core of the suggestion is breaking up the loop into the basic components of, in this case partial, iteration: increment and access.

The incrementing logic isn't just your `++'; the logic of iteration is bent by the conditions `i<4' and `it < vec.end()' so these conditions necessarily change the nature of the `continue' function.

Thanks to the dance of incrementing your access remains normal and trivial: `*it'.

Obviously, because you process the collection as a collection over major iteration, the partial processes, for example, deciding what three tetrahedrons to add, so returning a set of tetrahedrons, is a process treated as a separate component of processing the collection without managing the collection.

To be clear, no one has suggested arbitrarily breaking your code up into functions and passing around every stack variable as a reference. The goal isn't to do exactly what you've done in a different way; you must refactor, rewrite, the code so that the objects belonging to the collection are "fed", as necessary, into several district components each operating on as few as necessary tetrahedrons from the collection to do the job while the major loop drives how the functions are "fed" and what to do with the results.

I hesitate to call this a "state machine", but the label isn't far off.

Code:

try
{
    for( ;; )
        for( ;; )
            throw 1;
}
catch( int )
{    }

This example should never have been suggested.

@MutantJohn: I promise you, you would be fired for writing that C++ at any of the places I've worked.

Soma

Originally Posted by anduril462

Yep. It's so simple, even I can figure out how to use goto in places I don't need to (and shouldn't) use it.

And why exactly is that?

Originally Posted by phantomotap

This example should never have been suggested.

@MutantJohn: I promise you, you would be fired for writing that C++ at any of the places I've worked.

Please do enlighten me, O' self-righteous one.

Please do enlighten me, O' self-righteous one.

O_o

I've already told you.

(See the time you attempted to transform exceptions into references to be conditionally checked for "nullness".)

In short, you are using the exception mechanism poorly while making the code more complicated for absolutely no reason without meaningfully changing the problem of perception you are attempting to correct.

Soma

Originally Posted by phantomotap

O_o

I've already told you.

(See the time you attempted to transform exceptions into references to be conditionally checked for "nullness".)

In short, you are using the exception mechanism poorly while making the code more complicated for absolutely no reason without meaningfully changing the problem of perception you are attempting to correct.

Soma

I think you're mixing up my "null reference" and "unchecked return value throws exception" experiments. In any case, I don't see how throwing an exception is such a "poor" choice. Exceptions make great control flow constructs.

I'd also like to mention that if I was immediately fired for using a goto without being told to at least go back and rewrite my code then that'd be a very stressful job.

phantom, the reason why a goto was used was because it was a simple solution to integrate and yes, it's pedantic. I can't imagine using a non-ISO compliant compiler. People know g++ is free, right? Also, if a goto was the bane of all existence then why did the developers spend time creating it?

I think the first reply in this thread was right. It's not the best use of a goto but it's not bad. It doesn't break my code and for once, I can actually triangulate 8000 points whereas before I couldn't triangulate 4000 not using goto as a means of iteration. I like using goto in this context because it doesn't obfuscate my code from what it was before. Now, I will look into the use of stacks but in the meantime, a goto seems to suffice just fine. I will analyze the time it would take to rework the structure of my code and see if it's exactly that much better of an implementation to use stacks.

Also, my code really looks likes this :

Code:

for (auto it = fract.begin(); it < fract.end(); ) {

   test :

   tetra *t = *it;

   for (int i=0; i<4; i++) {

      if (t->ngb[i]) {

          auto distance = std::distance(fract.begin(), it);

          switch (x) {

               case 23 :

                     if (two_to_three(fract, inherit, t, t->ngb[i], plane, x, y) == 0)
                        it = fract.begin() + distance;
                     else
                        it++; 
                     if (it >= fract.end()) return;

                    goto test;
           }
      } if (i == 3) it++;
   }
}

This way, it's a bit more clear to see what I'm doing. I don't know... A goto was a simple solution to a problem that presented itself and so far, my code seems stable. You can use non-compliant compilers all you want and I'm not guaranteeing that my code will work. But as far as I can tell, the -pedantic flag is silent during compilation so it's ISO cleared and if you're compiler isn't ISO compliant then whose fault is that? I can't write for every possible scenario or odd compiler you use. Hopefully the VB compiler is ISO compliant...