Wednesday, October 10, 2007

Do you use goto in C#?

Looking through the code for the SmtpClient.Send method in Reflector and found this snippet:

switch (this.DeliveryMethod)
{
case SmtpDeliveryMethod.SpecifiedPickupDirectory:
if (this.EnableSsl)
{
throw new SmtpException(SR.GetString(
"SmtpPickupDirectoryDoesnotSupportSsl"));
}
break
;
case SmtpDeliveryMethod.PickupDirectoryFromIis:
if (this.EnableSsl)
{
throw new SmtpException(SR.GetString(
"SmtpPickupDirectoryDoesnotSupportSsl"
));
}
goto
Label_021C;
default:
goto Label_022B;
}
MailWriter fileMailWriter =
this.GetFileMailWriter(
this.PickupDirectoryLocation);
goto
Label_025D;
Label_021C:
fileMailWriter = this.GetFileMailWriter(
IisPickupDirectory.GetPickupDirectory());

goto
Label_025D;
Label_022B:
this.GetConnection();
fileMailWriter =
this.transport.SendMail((message.Sender != null) ?
message.Sender : message.From, recipients,
message.BuildDeliveryStatusNotificationString(),
out exception);
Label_025D:
this.message = message;
message.Send(fileMailWriter,
this.DeliveryMethod != SmtpDeliveryMethod.Network);
fileMailWriter.Close();

this
.transport.ReleaseConnection();
if
((this.DeliveryMethod == SmtpDeliveryMethod.Network) && (exception != null))
{
throw exception;
}

I've always been taught to avoid goto like the plague, and it's something I've followed for years. I've never even considered it to be an option when writing code.

Thinking about it though, I did read an article a few years ago (which I can't find now) which said you could credibly use gotos only if you used it to jump down code, and not up: a rule that is stuck to here.

The thing is, I know this code can be written without using goto. But also, it might not be as readable as it is now.

I'm conflicted. But I'm sticking to the "no goto" rule until someone can come up with an example that really can't be coded without using a goto.

7 comments:

Anonymous said...

As you know, I share many of your views on coding style and techniques and I would say that a loathing of the goto is part of that. Whilst I am sure that someone can come up with an example of somthing that requires it, it would still be a case of the exception rather than the rule; as you say, the example you show could be coded without using it.
I recall when the first drafts of the C# specification were made available there were a few comments made to the effect of what a pity they didnt choose to exclude this contentious feature!.
IMHO the problem with it is that it promotes spaghetti type code in which paths intertwine. To return to your example note how the execution in the second branch of the case jumps to the first label/code block and then to the third, while another jump initiated from the default branch ends up between the two.

It's also worth mentioning in a discussion about the goto is the fact that it is possible to effect one without the use of the word. How many times do you see returns from the middle of a function? Not an issue if it occurs say after validating an input [though throwing an exception might be better style]. But if is after allocating a load of resources then there may be potential for problems to arise - less so with garbage collection in C# but those of us who used C++ are still seem to be mindful of these things!!

Neil kilbride said...

Being the 'junior' here i'll keep my comments brief :P

I think in some rare scenarios, where there are lots of if statements, loops and flags, the GOTO command can help rather than hinder.

Also, it must be a benefit performance wise, especially if you are throwing exceptions (which we know are expensive) to jump around the code.

I don't think it should be completely avoided, just evaluate carefully what you are trying to do in code before using it. Saying all this, I've used GOTO only a few times - all at uni (which seems so long ago now).

Anonymous said...

The goto you see in reflector is probably just because the of the decompilation that reflector does. It doesn't mean that the original sourcecode looked like this, it just means that the MSIL code looks like this when represented as C#. I've seen this happen in reflector when viewing my own code too.

Akidan said...

I think there's such a knee-jerk reaction against goto sometimes, that people instantly dismiss it as a bad construct, regardless of how it's used.

One thing to remember is that all the loops and conditionals you use in C# (if, while, for, foreach, switch...) are ALL implemented using some form of goto (breaks in IL) behind the scenes.

For example, take the following code and compile it in an class library in Release mode.

class myclass
{
void a(object x, object y)
{
if ((x == null) && (y == null))
{
System.Console.WriteLine("foo");
}
else
{
System.Console.WriteLine("bar");
}
}

void b(object x, object y)
{
if (x != null) goto end;
if (y != null) goto end;

System.Console.WriteLine("foo");

return;

end: System.Console.WriteLine("bar");
}
}

Now, open this in Reflector and look at the IL and C# generated for method a versus method b...

Ian Dykes said...

I've made a response here

Anonymous said...

I see nothing wrong with using GOTO in a Switch statement.

Anonymous said...

Dont use switch either..


However i thing gotos are ok ( better than switch) when you have doubly nested loops