Does anyone have a good example/sample of “goto” spaghetti code? [closed]

Question

I've read a lot about how GoTo was considered harmful and removed for other control structures that were more intuitive. Does anyone have a good example / sample of goto spaghetti code? Preferrably, the sample code should be difficult to follow, but realtively easy when rewritten into more conventional control structures.

I know I could try to write you some of my own, but I've never really used goto and don't think I could due justice to the headaches its abuse can lead to.

I want this for didactic purposes to train junior developers on what to avoid. Mainly, to point to illustrate how OOP is taking the same idea to next logical consequence.

EDIT: by good example I mean code that is terrible to read and abuses it, rather than code that uses goto for reasonable optimization

Answer 1

Let's say the problem to solve (for example during an interview) is the following:

Given a text file, you must search for lines which match a pattern, given that bool MatchPattern(string) method expects only non-empty strings, while the file may contain empty lines. The file must contain at least one match.

Here's an example of a really bad approach which abuses goto.

class Example
{
    string match;

    public List<string> Matches;

    public void DoTheJob()
    {
        File data = null;
        try
        {
            data = File.Open(@"C:\Data.dat");
        }
        catch (Exception ex)
        {
            goto displayError;
        }

    readLine:
        // Go through all the lines of code.
        string line;
        try
        {
            line = data.ReadNextLine();
        }
        catch (Exception ex)
        {
            // File ended.
            if (this.Matches.Count == 0)
            {
                // This is not expected! The file must contain at least one match.
                goto displayError;
            }

            goto end;
        }

        if (line.Length == 0)
        {
        }
        else
        {
            // Good, there is something on this line.
            if (MatchPattern(line))
            {
                this.match = line;
                goto foundMatch;
            }
        }

        // Read the next line.
        goto readLine;

    end:
        return;

    displayError:
        MessageBox("ERROR!!!");

    foundMatch:
        this.Matches.Add(this.match);
        goto readLine;
    }
}

With four labels, the code becomes completely unreadable. At the first glance, thanks to the comments, we understand that it's about looping through the lines in a file, but then, it's not easy to understand what's happening here. Let's refactor it.

class Example
{
    public IEnumerable<string> ListMatches()
    {
        try
        {
            return this.TryListMatches();
        }
        catch (FileException)
        {
            // Log the error here and show something more explicit than a screaming "ERROR!!!" to the user, like a message inviting the user to check if permissions are set correctly for the file, or if the file exists, etc.
            ////throw; // Uncomment this if the exceptions must be rethrown.
        }
        catch (MatchNotFoundException)
        {
            // Log the error here and show a message explaining that the file seems to be invalid.
            ////throw; // Uncomment this if the exceptions must be rethrown.
        }
    }

    private IEnumerable<string> TryListMatches()
    {
        File data = File.Open(@"C:\Data.dat");
        IEnumerable<string> lines = data.ReadAllLines();

        bool foundMatch = false;
        foreach (var line in lines)
        {
            if (!string.IsNullOrEmpty(line) && this.MatchPattern(line))
            {
                foundMatch = true;
                yield return line;
            }
        }

        if (!foundMatch)
        {
            throw new MatchNotFoundException();
        }
    }
}

Now, it becomes obviously easy to understand what's going on. In some languages, like C#, you may go even further with functional programming paradigms, and replace the previous TryListMatches method by even shorter:

private IEnumerable<string> TryListMatches()
{
    var matches = File
        .Open(@"C:\Data.dat")
        .ReadAllLines()
        .Where(line => !string.IsNullOrEmpty(line) && this.MatchPattern(line));

    if (!matches.Any())
    {
        throw new MatchNotFoundException();
    }

    return matches;
}