Channels ▼
RSS

Design

Programming with Reason: Why is goto Bad?



In his 15+ years of developing software, Eric Bruno has acted as technical advisor, chief architect, and led teams of developers. He can be contacted at www.ericbruno.com.


I've been part of software teams and projects where the leaders start by putting together a "programming standards" document. The first entry always seems to be "don't use goto" if the language in question supports it. I'll start by saying this: Aside from BASIC programs I wrote in high school, I've never used the goto keyword in any of my code. In fact, with Java, there is no goto statement. Or is there?

Most programmers will agree that properly written, well-structured, code doesn't require the use of goto, and I agree with this. The problem is, if you simply state "no goto" you might as well say "no JMP command in the generated assembly code" as well. When we write and compile code, the result is assembly/machine code that the processor can execute. Even higher-level languages like Java get compiled to machine code when executed, thanks to technology such as the Hot Spot Java VM.

If you've ever written assembly code, or viewed the output of a compiler, you'll notice the JMP command all over the place, usually coupled with a test for some condition. This is one way that branching and code flow control is performed in the CPU. Basically, it's the same as a goto statement, and if you subscribe to the "no goto" rule, why not eliminate it also? The answer is, if you did, you probably couldn't get any of your software to run. I use this as proof that the "no goto" rule is flawed; we need to state something more concise.

It's More than Goto

I've seen a lot of code that violates "no goto" rule without even using goto. For instance, if goto violates the principals of structured code, does calling return in the middle of a method violate it also? I tend to say yes, and I try to structure my code to avoid this. Look at the following as an example:


    private boolean processTransaction(Transaction trans) {
        // do some session security checking
        User user = trans.user;
        log("security check for user " + user.name);
        if ( user.loggedIn() == false)
            return false;

        if ( user.sessionTimeout() == true )
            return false;

        boolean success = true;

        // process transaction here

        return success;
    }

This method checks two things before attempting to process a transaction: First, it checks that the user is logged in; and second, it checks that the user's session hasn't timed out. If either check fails, it aborts the transaction by returning false, but it does so in two places in the middle of the method. I refer to this as a mid-method return (MMR) and there's something I don't like about it. For instance, it's not always obvious where the checks begin and end, and the actual processing occurs.

In this case, the code above the checks stores the user object and logs the user's name. The code after the checks does the actual work of processing the transaction, but what if another programmer needs to modify this code? He could accidentally insert some code between the if statements, inadvertently doing some work that shouldn't be done until after all the checks are complete. In this simple example, it would be obvious to most programmers that this is wrong; but in a lot of code I've seen, it's easy to make this mistake. To resolve this, I propose that the following code is better:



    private boolean processTransaction(Transaction transaction) {
        // do some session security checking
        User user = trans.user;
        log("security check for user " + user);

        boolean success = false;
        if ( user.loggedIn() ) {
            if ( ! user.sessionTimeout() ) {
                // process transaction here
            }
        }
        
        return success;
    }

It's now more obvious where the checks are complete and the real processing begins; the code actually draws your eyes to the right spot. Also, with only one line in the method that returns a value, you have less "mental accounting" to do as you read from top to bottom. As a pleasant side effect, it's easy to discover where the success variable is declared. (How many times have you hunted through a long method to find declarations?)

Some people don't like nesting all of the if statements, but I find that this can usually be limited or avoided outright by combining related checks. The following code is an example:



    if ( user.loggedIn() && ! user.sessionTimeout() ) {
        // process transaction here
    }

Since the user's login and session timeout status are related checks, I have no problem checking them both in the same if statement. If there were a requirement to also check the date (i.e. check that it's not Sunday), then I'd prefer to see that in a separate if statement — but that's a matter of taste.

The point is that mid-method returns can be just as bad as using a goto statement in your code; they disrupt code structure, flow, and readability. However, that's not where it ends.


Related Reading


More Insights






Currently we allow the following HTML tags in comments:

Single tags

These tags can be used alone and don't need an ending tag.

<br> Defines a single line break

<hr> Defines a horizontal line

Matching tags

These require an ending tag - e.g. <i>italic text</i>

<a> Defines an anchor

<b> Defines bold text

<big> Defines big text

<blockquote> Defines a long quotation

<caption> Defines a table caption

<cite> Defines a citation

<code> Defines computer code text

<em> Defines emphasized text

<fieldset> Defines a border around elements in a form

<h1> This is heading 1

<h2> This is heading 2

<h3> This is heading 3

<h4> This is heading 4

<h5> This is heading 5

<h6> This is heading 6

<i> Defines italic text

<p> Defines a paragraph

<pre> Defines preformatted text

<q> Defines a short quotation

<samp> Defines sample computer code text

<small> Defines small text

<span> Defines a section in a document

<s> Defines strikethrough text

<strike> Defines strikethrough text

<strong> Defines strong text

<sub> Defines subscripted text

<sup> Defines superscripted text

<u> Defines underlined text

Dr. Dobb's encourages readers to engage in spirited, healthy debate, including taking us to task. However, Dr. Dobb's moderates all comments posted to our site, and reserves the right to modify or remove any content that it determines to be derogatory, offensive, inflammatory, vulgar, irrelevant/off-topic, racist or obvious marketing or spam. Dr. Dobb's further reserves the right to disable the profile of any commenter participating in said activities.

 
Disqus Tips To upload an avatar photo, first complete your Disqus profile. | View the list of supported HTML tags you can use to style comments. | Please read our commenting policy.
 

Video