CODEWITHSUNDEEP
c#loopsgoto
Trevor Thorpe
Trevor Thorpe

Reputation: 53

Is this a safe way to use goto?

foreach (var thing in things)
{
    tryagain:
    string thing.var1 = ThisCanReturnNullSometimes();
    if (thing.var1 == null)
    {
        goto tryagain;
    }
}

I know ideally you don't want a method that can "fail" but I'm working with the youtube data API and for some reason some calls just.. don't follow through.

It seems like a short and sweet way to reattempt the iteration, but i've never used a goto before and I've only heard people say don't use it.

Upvotes: 1

Views: 187

Answers (3)

Keith Nicholas
Keith Nicholas

Reputation: 44288

Your goto is safe, but is generally not used. Essentially you have written an implementation of a while loop.

But your code does have an interesting trait, your variable can be scoped and assigned and still be available after the loop... which can be concisely done like :-

tryagain: var s = ThisCanReturnNullSometimes();
if (s == null) goto tryagain;

however, while that is interesting.... I'd stick with a while loop or helper method if you'd like it more concise

Of course, it also has the additional problem of being an infinite loop in the case of null being returned all the time.

Upvotes: 0

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726559

Most programs can be expressed without goto. In this particular case, a loop is a far more readable construct, because it says pretty much what you want it to say:

string x;
do {
    x=CanReturnNullSometimes();
} while (x==null);

One nice thing about this loop is that the readers always know its post-condition: the only way this loop can terminate is that x becomes non-null. You can also add safety check to ensure that you are not calling the method too many times.

Upvotes: 4

Steve
Steve

Reputation: 9561

You would probably be better off using something like a while loop to monitor the status of your method and keep trying it. You could add a maximum iteration check to make sure it doesn't just loop forever.

string thing.var1 = ThisCanReturnNullSometimes();
int iteration = 0;
while (thing.var1 == null && iteration < 5)
{    
    Thread.Sleep(5000); // sleep for a bit to give the remove service time to "work"
    thing.var1 = ThisCanReturnNullSometimes();
    iteration++;
}

This will sleep for 5 seconds then try the method again and repeat up to 5 times before continuing on.

Of course, the best way would be to work out why your method fails, if it's a common problem or something that can be fixed.

Upvotes: -1

Related Questions