Why I used the ‘goto’ statement in C# today
Today I did something I have only ever done once before. I used the goto statement in C#.
In batch files I used goto all the time. For those who don’t know, LANDesk Management Suite supports batch file distribution packages and I became an expert at scripting with batch files. Of course, as a batch file scripting expert, I am very good at using the goto statement, but honestly using it in C# is generally frowned upon. I am sure there are other poeple like me that have used a language where goto is popular and can recognize uses for it when they crop up.
Anyway, I needed to start, stop, or restart a windows service and I need to perform the actions asynchronously in the background. Everytime I tried to create three methods, a Start, Stop, and Restart, the code grew in size and there was lots of duplication. So I kept going back to a single method that takes an action of start, stop, restart.
Since I wanted to run the action in the background, I created a ServiceWorker that inherits from BackgroundWorker and has an Action property (for Stop, Start, Restart) and here is the method I used.
private void StartOrStopService(object sender, DoWorkEventArgs doWorkEventArgs)
{
ServiceWorker worker = sender as ServiceWorker;
if (worker != null)
{
restart:
switch (worker.Action)
{
case "Start":
Log.WriteLine("Attempting to start service: " + worker.ServiceController.DisplayName);
worker.ServiceController.Start();
worker.ServiceController.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromMilliseconds(30000));
break;
case "Restart":
case "Stop":
worker.ServiceController.Stop();
worker.ServiceController.WaitForStatus(ServiceControllerStatus.Stopped, TimeSpan.FromMilliseconds(30000));
if (worker.Action == "Restart")
{
worker.Action = "Start";
goto restart;
}
break;
default:
return;
}
}
}
I of course have other options, but if I break this method out into more methods, it is actually more code. For example, if I make Start and Stop methods,
private void StartOrStopService(object sender, DoWorkEventArgs doWorkEventArgs)
{
ServiceWorker worker = sender as ServiceWorker;
if (worker != null)
{
switch (worker.Action)
{
case "Start":
StartService(worker);
break;
case "Stop":
StopService(worker);
break;
case "Restart":
StopService(worker);
StartService(worker);
break;
default:
return;
}
}
}
private void StartService(ServiceWorker worker)
{
Log.WriteLine("Attempting to start service: " + worker.ServiceController.DisplayName);
worker.ServiceController.Start();
worker.ServiceController.WaitForStatus(ServiceControllerStatus.Running, TimeSpan.FromMilliseconds(30000));
}
private void StopService(ServiceWorker worker)
{
Log.WriteLine("Attempting to stop service: " + worker.ServiceController.DisplayName);
worker.ServiceController.Stop();
worker.ServiceController.WaitForStatus(ServiceControllerStatus.Stopped, TimeSpan.FromMilliseconds(30000));
}
What’s your opinion?
Do you like 1 method with 28 lines including a goto statement?
Do you like 3 methods totaling 36 lines without the goto statement?
Leave a comment and let me know.
By the way, if you have read this post, you will see that I ended up eliminating the switch and the goto statements altogether.>
Eliminating Cylclomatic Complexity by replacing switch/case with a method or a Dictionary
I am definitely prefer second way. You have 3 functions, each responsible for a single task. It's much more readable and maintainable.
If you are worry about repeating code in StartService/ StopService, you can refactor them into one method Execute with extra parameters
E.g. String commandName="start", delegate command=Start(), newstatus=Running
I can see what you mean. From a testing perspective three methods is probably easier to test, even though it is more lines.
First, I'm no expert. I don't even use C#, because I've been using Basic since the '80s and when I switched career paths from Engineering to Programming, I was using VBA for AutoCAD (no C# option back then), and now VB.NET.
But it seems that if you really wanted to get rid of the goto, all you would have to do is change the goto line to recall the sub:
if (worker.Action == "Restart")
{
worker.Action = "Start";
StartOrStopService(worker, doWorkEventArgs);
}
Technically it would result in the first two lines of code being executed twice, but is exactly the same number of lines (or one less, if you count removing the restart: lable)
Since I've been using Basic since before it was Visual, I don't really have any particular aversion to GoTo, but after a quick search through my largest project, I have found a few that I was able to eliminate by rearranging the code a little. (Mostly I'm finding code that was written before I knew about the Using directive, and the goto was there so I can dispose objects before hitting the Next)
By they way, I personally see nothing wrong with either methods.