Re: Please, no knee-jerk "no goto ever" reactions

Posted Dec 25, 2014 23:08 UTC (Thu) by ldo (guest, #40946)
In reply to: Re: Wouldn't it be simpler and more readable to use a goto instead of a pseudo-loop? by vonbrand
Parent article: The "too small to fail" memory-allocation rule

Fine. Go and rewrite my code to use GOTOs, and see how you do.

Re: Please, no knee-jerk "no goto ever" reactions

Posted Dec 30, 2014 2:10 UTC (Tue) by filipjoelsson (guest, #2622) [Link] (2 responses)

I thought it would be a fun exercise to rewrite your code with the exact same structure that you used. But instead of the do { /* code */ } while (false); construct - I'd use goto cleanup. The point of this would have been to prove to you that what you are doing is in fact to use goto, but in a circumspect manner.

You do realize that your construct will optimize to exactly the same code, when compiled, right? So, that all you do is add empty words and pointless levels of indent, that may or may not help compilers and other programmers understand what you mean?

So I took a dive into your code, stripped out all the comments and newlines, ran it through indent, and tried again.

And I must say that I believe that do { /* code */ } while (false); is one of your smaller sins!

Why the f*ck do you for instance do things like this?

for (i=0;;) { 

if (i == nrcolors) break; 

/* code */ 

++i;

}

Breaking expectations like that do not anything of value, and makes sure nobody else will want to bother helping out on your project.

You also have nuggets like:

if (nrhistentries <= 4 || /* other condition */)

      {

        if (nrhistentries > 0) {

          histogram[0].index = 0;

          if (nrhistentries > 1) {

            histogram[1].index = 1;

            if (nrhistentries > 2) {

              histogram[2].index = 2;

              if (nrhistentries > 3) {

                histogram[3].index = 3;

              }

            }

          }

        }

Which I would like to replace with:


int max_histentries = (nrhistentries>4)?4:nrhistentries;

for (i=0; i<max_histentries; ++i) {

    histogram[i].index = i;

}

I also wonder what the point is of putting semicolons after comments.

In conclusion: I was in your camp until this discussion started - I really never ever use goto. You, however, have convinced me that there are reasonable exceptions to that rule.

Re: Why the f*ck do you for instance do things like this?

Posted Dec 30, 2014 20:42 UTC (Tue) by ldo (guest, #40946) [Link] (1 responses)

I have this convention: if one of the exits from a loop is via an explicit break, then all exits from the loop shall be via explicit breaks. That way you don’t have to look for more than one kind of termination condition, just look for the breaks. Only if there are none will there be something like a for-loop termination condition. Never mix the two.

Re: Why the f*ck do you for instance do things like this?

Posted Dec 30, 2014 21:12 UTC (Tue) by filipjoelsson (guest, #2622) [Link]

Then don't use a for-loop for that. Use a do-while-loop. A for-loop is for iterating until a certain condition is met. The compiler is built to warn on relevant error patterns. Since you use the for-loop in this unexpected way - no one but you can contribute to your project. In effect it's only so much dead code.

Is there any construct that you use as intended?

Re: Please, no knee-jerk "no goto ever" reactions

Posted Dec 30, 2014 3:00 UTC (Tue) by flussence (guest, #85566) [Link] (2 responses)

Your code already uses gotos - albeit each one rendered with an indented, gaudy costume of do-break-while-0 euphemism.

But the compiler can see your royal highness is stark naked. And so can we.

Re: Please, no knee-jerk "no goto ever" reactions

Posted Dec 30, 2014 9:47 UTC (Tue) by anselm (subscriber, #2796) [Link] (1 responses)

GOTO used to be a real problem when it was all that people had for control flow (think 1960s-era FORTRAN).

Right now with the proper discipline I don't have a huge problem with “goto” in C. As far as I'm concerned it is way less of a problem for program structure than the style that ldo is advocating. ldo's style also seems to presuppose that “cleanup” consists exclusively of undoing earlier things in reverse order; in general one might have to do things in an error branch that one might not have to do during normal flow, and sorting these cases out just serves to make the code even more convoluted on top of all the extraneous “do { … } while (0)” and “if (…) break” constructs.

I teach programming as my day job (some of the time, anyway), and I can guarantee that anybody who came to me with code like ldo's would get a very stern talking-to.

Re: in general one might have to do things in an error branch that one might not have to do during normal flow

Posted Dec 30, 2014 20:53 UTC (Tue) by ldo (guest, #40946) [Link]

I believe this is another instance of the “cleanup” versus “rollback” argument made elsewhere. See my comments about idempotency of deallocation routines and NULLing subsumed pointers.