Jump to content

  • Log In with Google      Sign In   
  • Create Account

To goto or not to goto?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
93 replies to this topic

Poll: To goto or not to goto? (149 member(s) have cast votes)

How often do you use goto statements?

  1. Never (92 votes [61.74%])

    Percentage of vote: 61.74%

  2. Very rarely (43 votes [28.86%])

    Percentage of vote: 28.86%

  3. Occasionally (9 votes [6.04%])

    Percentage of vote: 6.04%

  4. Quite often (1 votes [0.67%])

    Percentage of vote: 0.67%

  5. I have made it my quest to use them at least 5 times in each function (2 votes [1.34%])

    Percentage of vote: 1.34%

  6. The language I use doesn't have this keyword (2 votes [1.34%])

    Percentage of vote: 1.34%

Vote Guests cannot vote

#21 Juanxo   Members   -  Reputation: 189

Like
0Likes
Like

Posted 07 August 2011 - 12:15 AM


I guess I am a heretic, but I use goto in other situations that haven't been mentioned here. The most common is something like this:

for (int i=0; i<n; ++i) {
  switch (some_array[i]) {
    case 0:
      //...
      break;
    case 1:
      if (some_condition())
    	i=n;// DONE!!!
      break;
    //...
  }
}
//...



That's cheating xD

There are other loops, that can explain better the intentions:


do {
  switch (some_array[i]) {
    case 0:
      //...
      break;
    //...
    ++i;
  } while (i < n && !(some_array[i] == 1 && some_condition()));
}
//...


Sponsor:

#22 Drathis   Members   -  Reputation: 141

Like
2Likes
Like

Posted 07 August 2011 - 12:33 AM


I guess I am a heretic, but I use goto in other situations that haven't been mentioned here. The most common is something like this:

for (int i=0; i<n; ++i) {
  switch (some_array[i]) {
    case 0:
      //...
      break;
    case 1:
      if (some_condition())
    	i=n;// DONE!!!
      break;
    //...
  }
}
//...

Now you are just avoiding goto for the sake of avoiding goto. You didn't improve readability at all.

#23 Trienco   Crossbones+   -  Reputation: 2194

Like
0Likes
Like

Posted 07 August 2011 - 01:55 AM

Now you are just avoiding goto for the sake of avoiding goto. You didn't improve readability at all.


That seems to be a common thing. At one job I kept running into code that replaced gotos with while(1) break constructs, ie

if ( a_fails)
  goto cleanup;

if ( b_fails)
  goto cleanup;

turned into
while (1)
{
  if (a_fails)
	break;

  if (b_fails)
	break;

  break;
}

Of course this is even less readable and half the time people would forget the final break. The "optimized" version was using a "do while(false)" loop.

It's what happens when someone blindly follows the "thou shalt not goto"-chants by trying to emulate a goto with weird constructs instead of trying to design his code in a way that doesn't need a goto (or pseudo-goto) in the first place.

Yet, if I ever DO run into a situation in C++ where goto is the cleanest and most readable option, I will happily use it. Didn't happen in over 12 years, though.
f@dzhttp://festini.device-zero.de

#24 szecs   Members   -  Reputation: 2148

Like
1Likes
Like

Posted 07 August 2011 - 02:34 AM

I think the ones against goto always use the worst examples of gotos. I don't really get the "intent of code is not clear" argument (that case example in this thread was ironically a pretty good example that avoiding that goto made the code less clear and more error-prone. What if I wanted to examine 'i' after the loop? what if I wanted to add some stuff after the switch but inside the loop later?).

Intent of code is not clear when used in the most idiotic ways (like long jumps (code-wise), jumping back and forth, variable declarations messed with gotos, replacing more sophisticated constructs with gotos, having too big functions, jumping into loops/conditional-blocks maybe even jumping in and out of functions). Other than those, it's so clear for me: "go to that line". If you have a small function (which you should) and have some other "rules" of use (like label is after goto, no variable declarations between the two if they are in the same scope) then I can't think of any clearer ways to control program flow. It's just as clear as "break" for example (some argue that break is evil too. Well, good luck with the 5th 'foo' variable just to control flow...). "Continue" is a much more unclear solution IMHO.

Of course, these stuff should not be in the "framework" code. I don't know the proper terms. So the main loops, or "high level stuff" shouldn't have these. This high level stuff should be well designed and should be awesome. But I can't really think of a way to write "neat" or "well designed" code for a complex algorithm for example (before you bring up the "design it better" argument).

Okay, I'm a person who doesn't really give a shit about the implementation of "black box" (pure) functions. If they work, it's time to move on. I don't give a fuck (and I wouldn't give a fuck if I worked in a team) how that function is implemented if it's not my task to implement it. All I care about its interface and performance.

#25 Dragonion   Members   -  Reputation: 131

Like
0Likes
Like

Posted 07 August 2011 - 04:39 AM

I think the ones against goto always use the worst examples of gotos.

And for that reason, here is an example where I personally think it makes good sense to use goto statements (like a replacement for a small inline function):
static MyClass*MyClass::createInstance(void)
{
	MyClass*inst=null;

	if(!(inst=new MyClass()))
    	return null;
	if(!(inst->object1=new Object1()))
    	goto FAILED;
	if(!(inst->object2=new Object2()))
    	goto FAILED;
	if(!(inst->object3=new Object3()))
    	goto FAILED;
	if(!(inst->object4=new Object4()))
    	goto FAILED;

	return inst;

	FAILED:
	{
    	delete inst->object1;
    	delete inst->object2;
    	delete inst->object3;
    	delete inst->object4;
    	delete inst;

    	return null;
	}
}

Surely you could avoid using them like this:

static inline void MyClass::failed(MyClass*inst)
{
	if(inst)
	{
    	delete inst->object1;
    	delete inst->object2;
    	delete inst->object3;
    	delete inst->object4;
    	delete inst;
	}
}
static MyClass*MyClass::createInstance(void)
{
	MyClass*inst=0;

	if(!(inst=new MyClass()))
    	return null;

	if(!(inst->object1=new Object1()))
	{
    	failed(inst);
    	return null;
	}
	if(!(inst->object2=new Object2()))
	{
    	failed(inst);
    	return null;
	}
	if(!(inst->object3=new Object3()))
	{
    	failed(inst);
    	return null;
	}
	if(!(inst->object4=new Object4()))
	{
    	failed(inst);
    	return null;
	}

	return inst;
}

... but in this case I honestly think the goto version is the more elegant/clean solution.

PS: I know you should use the 'static' keyword in the function declaration/prototype, not in the definition/implementation, but I inserted them here to avoid making a class definition just to show that these functions are static.

EDIT: Or perhaps a combination providing the opportunity to re-use the delete functionality:
static inline void MyClass::deleteInstance(MyClass*inst)
{
	if(inst)
	{
    	delete inst->object1;
    	delete inst->object2;
    	delete inst->object3;
    	delete inst->object4;
    	delete inst;
	}
}
static MyClass*MyClass::createInstance(void)
{
	MyClass*inst=null;

	if(!(inst=new MyClass()))
    	return null;
	if(!(inst->object1=new Object1()))
    	goto FAILED;
	if(!(inst->object2=new Object2()))
    	goto FAILED;
	if(!(inst->object3=new Object3()))
    	goto FAILED;
	if(!(inst->object4=new Object4()))
    	goto FAILED;

	return inst;

	FAILED:
	{
    	deleteInstance(inst);
    	return null;
	}
}


#26 Antheus   Members   -  Reputation: 2397

Like
0Likes
Like

Posted 07 August 2011 - 05:31 AM

And for that reason, here is an example where I personally think it makes good sense to use goto statements (like a replacement for a small inline function):


Better:
static MyClass*MyClass::createInstance(void)
{
	auto_ptr<Object1> a = new Object1();
	if (a->get() == 0) return 0;

	auto_ptr<Object2> b = new Object2();
	if (b->get() == 0) return 0;

	auto_ptr<Object3> c = new Object3();
	if (c->get() == 0) return 0;

	auto_ptr<Object4> d = new Object4();
	if (d->get() == 0) return 0;

	auto_ptr<MyClass> e = new MyClass();
 	if (e->get() == 0) return 0;
 	
	e->a = a->release();
	e->b = b->release();
	e->c = c->release();
	e->d = d->release();

	return e->release();
}
If using exceptions, the above gets simpler:
static MyClass*MyClass::createInstance(void)
{
	auto_ptr<Object1> a = new Object1();
    auto_ptr<Object2> b = new Object2();
 	auto_ptr<Object3> c = new Object3();
 	auto_ptr<Object4> d = new Object4();
 	auto_ptr<MyClass> e = new MyClass();
	
	e->a = a->release();
	e->b = b->release();
	e->c = c->release();
	e->d = d->release();

	return e->release();
}

auto_ptr is also apparently deprecated, I believe it's called unique_ptr<> these days.

#27 Dragonion   Members   -  Reputation: 131

Like
0Likes
Like

Posted 07 August 2011 - 05:55 AM

static MyClass*MyClass::createInstance(void)
{
	auto_ptr<Object1> a = new Object1();
	if (a->get() == 0) return 0;

	auto_ptr<Object2> b = new Object2();
	if (b->get() == 0) return 0;

	auto_ptr<Object3> c = new Object3();
	if (c->get() == 0) return 0;

	auto_ptr<Object4> d = new Object4();
	if (d->get() == 0) return 0;

	auto_ptr<MyClass> e = new MyClass();
 	if (e->get() == 0) return 0;
 	
	e->a = a->release();
	e->b = b->release();
	e->c = c->release();
	e->d = d->release();

	return e->release();
}

But what if the creation of, say, object 'd' (Object4) fails? Unless I am missing something, in this case object 'a',' b', and 'c' will not be de-allocated (which could cause a memory leak)?

#28 mhagain   Crossbones+   -  Reputation: 8000

Like
1Likes
Like

Posted 07 August 2011 - 06:08 AM

There are other loops, that can explain better the intentions:


do {
  switch (some_array[i]) {
    case 0:
      //...
      break;
    //...
    ++i;
  } while (i < n && !(some_array[i] == 1 && some_condition()));
}
//...


It's still not as clear as the goto version. The goto version has two major advantages over both alternatives presented:

(1) It puts the "get out of here" action in the same place as the condition that triggered the action.
(2) It's easier and clearer to add other "get out of here" conditions in the future without creating an overly complex condition on the while.

These are in addition to the "what if you need to put more code after the switch" point mentioned earlier.

Personally I also use goto for optimizing recursion. Recursive functions can be a bitch - lots of stack usage, heavy CPU overhead, risk of runaway, often used in parts of code (like scene traversal) that can generate the biggest bottlenecks. You can often rewrite a recursive function quite cleanly with goto and avoid all of this.

The point about goto though is that it's a valid tool if you know what you're doing. There is an extreme viewpoint being propagated here, which is that using any goto at all will instantaneously transform your code into old-school BASIC-style spaghetti, and therefore you should never ever use goto. It should be obvious that's nonsense. Use a tool where it's appropriate, don't use it where it's not, but know what you're doing (and why you're doing it) in both cases.

It appears that the gentleman thought C++ was extremely difficult and he was overjoyed that the machine was absorbing it; he understood that good C++ is difficult but the best C++ is well-nigh unintelligible.


#29 Antheus   Members   -  Reputation: 2397

Like
0Likes
Like

Posted 07 August 2011 - 06:28 AM

But what if the creation of, say, object 'd' (Object4) fails? Unless I am missing something, in this case object 'a',' b', and 'c' will not be de-allocated (which could cause a memory leak)?


How about looking up std::auto_ptr :P

If not using exceptions:
- constructor cannot fail (or will set a flag which needs to be checked)
- if allocation fails, pointer will be null

If using exceptions:
- constructor will throw exception
- allocation failure will throw std::bad_alloc

I'm going to go one step further with above example. If avoiding exceptions and not using C++ class life-cycle, we're back to C anyway, so let's use macros:
#define MUST_CREATE(a, b)
auto_ptr<b> a = new b();
if (a->get() == 0 || a.fail) return 0;

...
{
  MUST_CREATE(a, Object1);
  MUST_CREATE(b, Object2);
  MUST_CREATE(c, Object3);
  MUST_CREATE(d, Object4);
  MUST_CREATE(e, MyClass);

  e->a = a->release();
  e->b = b->release();
  e->c = c->release();
  e->d = d->release();

  return e->release();
}
Code not using exceptions is fairly limited in the extent of using constructors, so the creation will need a convention anyway, might as well make it as clear as possible.

A better question here is what the goal of such construction is. Avoiding exceptions for sake of poor compiler is increasingly rare and allocations from pool where allocations are commonly expected to fail would be done differently and not rely on bad_alloc.


The point about goto though is that it's a valid tool if you know what you're doing.


goto is "unconditional jump" or "unconditional branch".

If algorithm needs it and it isn't better expressed using one of built-in constructs (if/do/for/while), then it's obviously the right tool for the job.

Since majority of languages today emphasize scoping or nesting, jump becomes very limited in where it can be used.

#30 Dragonion   Members   -  Reputation: 131

Like
0Likes
Like

Posted 07 August 2011 - 07:03 AM

How about looking up std::auto_ptr :P

Hmm ... in that case, I admit your version is actually pretty sleek ;) My experience with STL is somehow mixed so I am always a little skeptical using it, but I just disassembled a small program using auto-pointers, and apparently they only yield a few more instructions compared to the 'standard approach'.

#define MUST_CREATE(a, b)
auto_ptr<b> a = new b();
if (a->get() == 0 || a.fail) return 0;

Beautiful! (except you forgot backslashes to split the lines)

#31 Bregma   Crossbones+   -  Reputation: 5182

Like
0Likes
Like

Posted 07 August 2011 - 07:25 AM

... If resources must be freed, then a function should be created to free resources and it should be called. In this way, all the freeing of resouces occurs in a single place, so if there is a memory leak, you should be able to find it quickly. A goto statement is bad because it means many more lines of code will have to be altered if a program flow changes. If the goto's are removed, minimal code changes are required because the program is separated into logical parts which are easy to modify on their own.

This is rather naive. Consider the following "constructor" in object-oriented C.
SomeObject *
some_object_new()
{
  SomeObject *some_object = calloc(1, sizeof(struct SomeObject));
  if (!some_object)
  {
    fprintf(stderr, "error allocating SomeObject\n");
    goto final_exit;
  }

  some_object->foo = foo_new();
  if (!some_object->foo)
    goto rollback_some_object;

  some_object->bar = bar_new();
  if (!some_object->bar)
    goto rollback_foo;

  some_object->baz = baz_new(some_object->bar);
  if (!some_object->baz)
    goto rollback_bar;

  goto final_exit;

rollback_bar:
  bar_delete(some_object->bar);
rollback_foo:
  foo_delete(some_object->foo);
rollback_some_object:
  free(some_object);
  some_object = NULL;
final_exit:
  return some_object;
}
Can you tell me that it would be better to have a series of functions, each chained to call the next, to simply call the deleter of objects during rollback? Would it instead be better to have ever-growing lists of things to roll back after each failure check? Or, what is commonly done, simply ignore failure and leak or crash? What metric are you suing to just the above code more unreadable or less maintainable than any alternative way of implementing the same functionality with the use of gotos?

I realize that people will still use goto's, but it leads to more confusion and bugs than it helps. Ask anyone who has worked on large projects before if the use of goto is a help or a hindrance and I know it will be that goto's do not help in any case what so ever. Programming is about structure and organization, and goto's really do break that concept.

Again, do you have metrics or are you simply accepting the word of gospel on the topic?

If a person is working on their own code by themselves, use goto all you want, but if you are working on a project with others, do not use goto. I agree with
Telastyn when he said, "If you used a goto in production code I would fire you on the spot." This is soo true

There are plenty of excellent Linux kernel developers who would be a fine addition to many teams. Not your teams, of course, because they aren't in the choir during the sermons, but successful, profitable teams involved with large successful projects. Ditto the Windows and Mac driver developers I've worked with.
Stephen M. Webb
Professional Free Software Developer

#32 YellPika   Members   -  Reputation: 142

Like
1Likes
Like

Posted 07 August 2011 - 07:28 AM

If I see a goto, I figure it's time to refactor.

Ex.
for (int i=0; i<n; ++i) {
  switch (some_array[i]) {
    case 0:
      //...
      break;
    case 1:
      if (some_condition())
        i=n;// DONE!!!
      break;
    //...
  }
}
//...

becomes

for (int i=0; i<n; ++i) {
  if (!bodyFunc(i))
    break;
}
//...

void bodyFunc(int i) {
  switch (some_array[i]) {
    case 0:
      //...
      return true;

    case 1:
      if (some_condition())
        return false;// DONE!!!

      return true;
    //...
  }
}

OK, not a goto, but you see the point :D

#33 smasherprog   Members   -  Reputation: 432

Like
0Likes
Like

Posted 07 August 2011 - 08:17 AM

I like your example better than mine YellPika. Good catch. You are right about refactoring.I like this discussion, so far no one is flaming anyone. . . . . . yet.
Wisdom is knowing when to shut up, so try it.
--Game Development http://nolimitsdesigns.com: Reliable UDP library, Threading library, Math Library, UI Library. Take a look, its all free.

#34 Tachikoma   Members   -  Reputation: 552

Like
0Likes
Like

Posted 07 August 2011 - 09:09 AM

Garbage collection of dynamically allocated memory is the only reason why I use goto statements in C, particularly when there are several if-statement failure points in the function. The intent is clear and easy to read.
Latest project: Sideways Racing on the iPad

#35 Antheus   Members   -  Reputation: 2397

Like
0Likes
Like

Posted 07 August 2011 - 10:30 AM

If I see a goto, I figure it's time to refactor.

Ex.

becomes

OK, not a goto, but you see the point :D


Refactoring should focus on clarity of intent, not just shuffling lines around.

bool acceptable(T x); // stateless, checks the value

for (int i=0; i<n; ++i) {
  if (!acceptable(some_array[i]) break;
}

C++ now allows to use common semantics, adding containers improves ranges as well:
iterator result = find_if(some_array.begin(), some_array.end(), not_acceptable);
if (result != some_array.end() {
}
Which makes code quite idiomatic and reduces all user-written code to one single stateless predicate reducing the mental load needed understand the code.

Transformation makes sense since it improves clarity of code. By defining predicate "acceptable" or "not acceptable" the intent is clear. Predicates should be formed in such a way to naturally fit with algorithms, which may also be reinforced by naming. So instead of "acceptable" or "not_acceptable":

bool should_be_removed(T x) {
  switch (x) {
	case 6 : return true;
	case 20 : return true;
  }
  return false;
}
The original code isn't complicated due to gotos, it simply mixes too many orthogonal design concepts.


The above applies increasingly to Java and especially C# (LINQ) as well as languages with closures or clojure-like concepts. Java and C# do not make this obvious since they require everything to be an object hinting that there should be state involved, whereas ideal predicates should be stateless and operate on values only.


In example given above, goto is indeed an indicator of code smell. It will make reviewer wonder if author actually understood the problem they are solving and why they didn't recognize it's a searching problem tends to be covered by just about every standard library. it will also require thorough review of the code to determine why it doesn't use common helpers which hampers maintainability. Principle of least surprise - if doing find_if, do that.

#36 Álvaro   Crossbones+   -  Reputation: 13363

Like
2Likes
Like

Posted 07 August 2011 - 08:52 PM

If I see a goto, I figure it's time to refactor.


I respectfully disagree. If `bodyFunction' in your code can be given a good name and a clean interface, I agree that that's the way to go. But if you end up with a function with some horrible name that needs to get passed a whole bunch of the local variables of the original function, I'll keep my goto, thank you very much.

Several of you seem to be assuming that what the code is trying to do is something trivial (finding a particular element, or something like that). If that were the case, of course it would be better to make the code more expressive. But that's not always the case.

#37 Telgin   Members   -  Reputation: 200

Like
0Likes
Like

Posted 08 August 2011 - 08:45 AM

I believe the only time I've used a goto in anything other than some BASIC language was when I was writing a C program that I needed to test in a Valgrind profiler I wrote to make sure the profiler would deal with the goto correctly (just in case someone in our research group decided to use a goto, which I doubt any have).

I'm not dead set on not using them, it's just that I've yet to really run into a place that I thought it was the best solution. In C++, for example, exceptions can deal with most cases of where you would want to use a goto. C doesn't have exceptions, but the C code I write is generally simple enough that it's not an issue. The only time I recall considering using a goto in C was where I had a four level nested looping structure, which was just indicative that I needed to find a better way to solve my problem, which I did.

Perhaps if I was writing some much more complex C code I'd have found more legitimate use for gotos (supposedly the Linux kernel contains them in some places, but I'll admit to having never looked).
Success requires no explanation. Failure allows none.

#38 kdmiller3   Members   -  Reputation: 176

Like
0Likes
Like

Posted 08 August 2011 - 09:52 AM

while (1)
{
	if (a_fails)
		break;

	if (b_fails)
		break;

	break;
}

This construct eliminates the risk of an infinite loop:
do
{
	if (a_fails)
		break;

	if (b_fails)
		break;

	// do stuff
}
while (false);
I'm still not fond of it, though. :)

#39 Vortez   Crossbones+   -  Reputation: 2698

Like
1Likes
Like

Posted 08 August 2011 - 09:28 PM

I use them very rarely, but still, if that make the code cleaner and easier to read and maintain, i prefer using a goto then messing the code up just for the sake of avoiding a goto...

#40 NightCreature83   Crossbones+   -  Reputation: 2832

Like
0Likes
Like

Posted 09 August 2011 - 06:18 AM


If I see a goto, I figure it's time to refactor.


I respectfully disagree. If `bodyFunction' in your code can be given a good name and a clean interface, I agree that that's the way to go. But if you end up with a function with some horrible name that needs to get passed a whole bunch of the local variables of the original function, I'll keep my goto, thank you very much.

Several of you seem to be assuming that what the code is trying to do is something trivial (finding a particular element, or something like that). If that were the case, of course it would be better to make the code more expressive. But that's not always the case.


In the case where the function isn't trivial I would try to avoid your goto as it adds additional complexity to the function.

Seeing GOTO's come with extremly strict rules on their usage which are developer enforced, it is generally beter to avoid them. I have never used a goto in my code ever and I don't think our code base has any gotos inside of it.
Worked on titles: CMR:DiRT2, DiRT 3, DiRT: Showdown, GRID 2, Mad Max




Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS