Should I use goto statement?

Question

I have a piece of code like the following:

try
{
Work:

   while(true)
   {
      // Do some work repeatedly...
   }
}
catch(Exception)
{
   // Exception caught and now I can not continue 
   // to do my work properly

   // I have to reset the status before to continue to do my work
   ResetStatus();

   // Now I can return to do my work
   goto Work; 
}

Are there better alternatives compared to using goto? Or is this a good solution?

Why not put your try catch inside the while? Then no need for the goto... — Chris, Nov 07 '12 at 17:12
I think it was downvoted, because someone mentioned Jeho ... i mean goto ;) Alot of people hate goto with a passion, while its almost never necessary or wise to use, it is still there for a reason and it is not the most evil statement. — dowhilefor, Nov 07 '12 at 17:14
@AlexK., i didn't downvote, but i did vote to close. there is no real answer to the question as it is currently phrased. lots of people can give you opinions on best practices, and most people will hat goto's, but in general, this is an opinion question. — nathan gonzalez, Nov 07 '12 at 17:14
@AlexK.: I assume because it is basically code review which is not what this site is about. Though I say that not as one of the downvoters but I considered a vote to close for that reason. — Chris, Nov 07 '12 at 17:14
If you write your code properly, you shouldn't need `goto` 99.999 percent of the time. — Robert Harvey, Nov 07 '12 at 17:15
@RobertHarvey: I've yet to find any case where a `goto` is REALLY cleaner. — LightStriker, Nov 07 '12 at 17:17
@RobertHarvey It's actually proven that you can refactor anything with a goto into a non goto based solution, so you *never* need it. As for whether you *should* refactor it out, that's a subjective matter. — Servy, Nov 07 '12 at 17:17
oh, and you should probably be catching more specific exceptions and dealing with them specifically. Catching general exceptions except for debug/logging is generally a pretty poor idea. Eg if you have networking stuff in there and your computer hasn't got a network then you're going to get into a nasty cycle where you probably actually want to just bomb out at that point or at least do something more useful than an immediate retry. — Chris, Nov 07 '12 at 17:19

Jon Skeet · Accepted Answer · 2012-11-07T17:35:04.510

16

It sounds like you really want a loop. I'd write it as:

bool successful = false;
while (!successful)
{
    try
    {
        while(true)
        {
            // I hope you have a break in here somewhere...
        }
        successful = true;
    }
    catch (...) 
    {
        ...
    }
}

You might want to use a do/while loop instead; I tend to prefer straight while loops, but it's a personal preference and I can see how it might be more appropriate here.

I wouldn't use goto though. It tends to make the code harder to follow.

Of course if you really want an infinite loop, just put the try/catch inside the loop:

while (true)
{
    try
    {
        ...
    }
    catch (Exception)
    {
        ...
    }
}

edited Nov 07 '12 at 17:35

answered Nov 07 '12 at 17:13

Jon Skeet

1,421,763
867
9,128
9,194

4

@Fake.It.Til.U.Make.It: I have no idea what you mean, I'm afraid. – Jon Skeet Nov 07 '12 at 17:18
1

That doesnt seem equivalent, he has while(true) which would not exit on success, unless I'm missing something. – heisenberg Nov 07 '12 at 17:20
This is the only sensible answer here. Congrats on the 500K. – Robert Harvey Nov 07 '12 at 17:20
he is calling the label `Work` which in `try` block from the `catch` block..is it **valid** – Anirudha Nov 07 '12 at 17:20
@Fake.It.Til.U.Make.It: No, it's not valid - but the OP could move it to just *before* the try block, which would make it valid but still ugly. – Jon Skeet Nov 07 '12 at 17:22
@Fake.It Just say no to `goto`. – Robert Harvey Nov 07 '12 at 17:22
@Robert I'm pretty sure its not equivalent to the example given in the question. – heisenberg Nov 07 '12 at 17:26
1

@kekekela Exact code equivalence has never been a requirement for answers, and the OP hasn't provided enough information to divine what the exact intent of his original code is. – Robert Harvey Nov 07 '12 at 17:28
@robert The guy wants a loop that keeps running indefinitely, he's given one that exits on first successful iteration, I don't really see how you can call that a correct answer but w/e – heisenberg Nov 07 '12 at 17:31
@Robert yeah, his code is bad...but the way to do what he wants to do correctly is what is laid out in Alexei's answer. – heisenberg Nov 07 '12 at 17:34
@kekekela: I've edited to make it clearer what the `...` was (it was meant to be *all* the code within the original `try` block, i.e. the `while(true)` loop, which would make it genuinely equivalent to the original). I've also added another equivalent version which just moves the try block. – Jon Skeet Nov 07 '12 at 17:36
Removed my downvote, congrats on 500,002 ;) – heisenberg Nov 07 '12 at 17:38

score 10 · Answer 2 · answered Nov 07 '12 at 17:12

10

Goto is very rarely appropriate construct to use. Usage will confuse 99 percent of people who look at your code and even technically correct usage of it will significantly slow down understanding of the code.

In most cases refactoring of the code will eliminate need (or desire to use) of goto. I.e. in your particular case you can simply mover try/catch inside while(true). Making inner code of the iteration into separate function will likely make it even cleaner.

while(true)
{
  try
  {
      // Do some work repeatedly...
  }
  catch(Exception)
  {
   // Exception caught and now I can not continue 
   // to do my work properly

   // I have to reset the status before to continue to do my work
   ResetStatus();
  }
}

answered Nov 07 '12 at 17:12

Alexei Levenkov

98,904
14
127
179

That's not an equivalent to Nick's code. His code was exiting on success. – Snowbear Nov 07 '12 at 17:15
@snowbear: I assumed the while(true) was keeping things in an infinite loop. Its just as easy to exit the while loop in this new form though... – Chris Nov 07 '12 at 17:16
1

It looks equivalent to me, how is while(true) exiting on success? – heisenberg Nov 07 '12 at 17:19
@Snowbear: Where does it exit on success? The only way to exit the loop is because an exception is thrown, which will lead to goto, then to the label and right back into the loop. Am I missing something? – Sebastian Negraszus Nov 07 '12 at 17:20
@Snowbear, I'm not sure where original sample exits on success... copy-pasted from the question as is - could have miss intention as Jon's answer suggests... – Alexei Levenkov Nov 07 '12 at 17:20
@SebastianNegraszus Yes, you could still `break`, or `return` out of the loop, but that could happen before or after the refactor, so the answer is perfectly valid. – Servy Nov 07 '12 at 17:21
@Servy: Well, It could. But there is no such thing in the given code, nor is it mentioned. – Sebastian Negraszus Nov 07 '12 at 17:22
@SebastianNegraszus We have no idea what `// Do some work repeatedly...` contains. It may or may not break out of the loop. My point is that it's possible, but still irrelevant. – Servy Nov 07 '12 at 17:23
@AlexeiLevenkov yes, the `while(true)` is `while(HaveToContinue)` and the `Exception` is a `SocketException`, but +1 anyway. – Nick Nov 07 '12 at 17:26

score 0 · Answer 3 · answered Nov 07 '12 at 17:15

0

It would seem to make more sense to just move the try/catch into the while loop. Then you can just handle the error and the loop will continue as normal without having to route control flow with labels and gotos.

answered Nov 07 '12 at 17:15

heisenberg

9,665
1
30
38

score -2 · Answer 4 · edited May 23 '17 at 12:03

-2

Catch and restore the state on each iteration, even though catching outside will work the same, here you are still in the loop and you can decide whether you want to continue or break the loop.

ALSO: Catching an Exception is wrong from the beginning (what are you going to do if you catch StackOverflowException or MemoryLeachException - EDIT: this is just for example, check documentation to know what you can catch in reality). Catch concrete type of exception which you expect to be thrown.

while(true)
{
    try
    {
        // Do some work repeatedly...
    }
    catch(FileNotFoundException) //or anything else which you REALLY expect.
    {
        // I have to reset the status before to continue to do my work
        ResetStatus();
        //decide here if this is till OK to continue loop. Otherwise break.
    }
}

for those very smart in the comments: Why don't catch general exception

edited May 23 '17 at 12:03

Community

1
1

answered Nov 07 '12 at 17:16

Tengiz

8,011
30
39

1

Actually a StackoverflowException can't be caught in the first place, just like a few other truly fatal exceptions (i.e. OOM). – Servy Nov 07 '12 at 17:18
That was just an example. Catching an Exception is wrong - that's what I meant. – Tengiz Nov 07 '12 at 17:19
I disagree on that point, but it's irrelevant to the question at hand so I see no point in discussing it in comments here. – Servy Nov 07 '12 at 17:20
Please read more on why shouldn't you catch general exceptions. I have just updated the answer with link. Enjoy your genuine way of thinking! – Tengiz Nov 07 '12 at 17:23
2

Not downvoted yet but... This should be comment as it is absolutely not related to the question (the sample code often have to be changed to make it small and show the issue)... – Alexei Levenkov Nov 07 '12 at 17:23
I think this answer gives good advice. But each person can make corresponding choice of understanding or not. – Tengiz Nov 07 '12 at 17:26
@Tengiz I am quite familiar with the argument, I simply disagree with it. Again, I'm not going to argue the point with you here as it's 100% off topic. – Servy Nov 07 '12 at 17:26
@Servy no problem at all. We both know what's right and what's not. – Tengiz Nov 07 '12 at 17:27
Probably now the answer looks better. I just changed the order of paragraphs. Even though it was first saying the worse thing I saw (my personal opinion), and the concrete answer next. – Tengiz Nov 07 '12 at 17:32

Should I use goto statement?

4 Answers4

Linked