Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question

closed as not constructive by gnat, Ryathal, ChrisF Jul 6 '12 at 14:57

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.If this question can be reworded to fit the rules in the help center, please edit the question.

    
I'm sure Edsger has a few great ones... –  Yannis Rizos Jul 6 '12 at 14:46
1  
Better show them nice, clean and decent uses of goto: for example, OCaml bytecode interpreter, or Knuth's adventure game. Or simply grep through Linux sources and see all the hundreds of the decent goto use cases. So please, do not mutilate the young brains, and stop spreading this anti-goto madness. And, btw, OOP is not such a "step forwards" as you probably think. –  SK-logic Jul 6 '12 at 14:49
    
Historically in man languages, if one wanted to do something as simple as if (x>y) x=y, one would have to say something like "IF X>Y THEN 4390", where 4390 was a label given to a line containing "X=Y". The line after 4390 would be "GOTO 3860", where 3860 would be a label given to a line after the if. It's not hard to imagine why that would lead to messy code. –  supercat Feb 14 at 22:02
    
@supercat I understand why, it's just that I'm looking for an example of it to show to students that seems unintelligible and then show them the structured version that is much more understandable to illustrate the point. –  ArtB Feb 16 at 0:32
    
@ArtB: Try atariarchives.org/morebasicgames (many of the games there were originally written in HP BASIC; they've been adapted to MS-Basic but the structure is largely original). –  supercat Feb 16 at 0:39

1 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;
}
share|improve this answer

Not the answer you're looking for? Browse other questions tagged or ask your own question.