70
$\begingroup$

A fellow teacher of mine is teaching "basic algorithms" course in high school, and we have quite conflicting opinions when it comes to returning from functions.

She restricts return only to the end of the function, and permits no other way to escape functions. No block guards, too, and no breaking loops using return.

To be clear, the approach she teaches her student with is "Single Entry, Single Exit", when the exit is at the function's end and does not break a loop (instead she prefers using flags, even though sometimes in my opinion it makes the code messier).

In contrast, I prefer teaching my students the option to return early and how it affects their code and flow, and let them decide by themselves what they think is right to do. The criteria guidelines I usually teach are:

  • Do you return early because of your functions logic, or because you were given an exceptional parameter/erroneous parameter? If we're talking about the latter, it is ok to return.
  • Are you able to avoid returning in the middle without the cost of changing your whole control flow? If not, it is ok to return.
  • Will avoiding returning early add unnecessary nesting levels to the function? If so, it is ok to return.

Is it a wrong approach, giving your students the choice? Or is it wrong to force them never to return early?

Edit the answers suggested some very interesting difference between many languages. We are teaching C#, which is a high level language, so I wasn't considering jmps of assembly or lower level C's deallocation of resources.

$\endgroup$
  • 2
    $\begingroup$ (Highly?) relevant question from StackOverflow: stackoverflow.com/questions/36707/… $\endgroup$ – tonysdg Mar 10 '18 at 1:28
  • 8
    $\begingroup$ @tonysdg Maybe (Highly?er) relevant answer from Software Engineering: softwareengineering.stackexchange.com/a/118793/275869 $\endgroup$ – Gypsy Spellweaver Mar 10 '18 at 14:41
  • $\begingroup$ Comments are for suggesting improvments on the question or requesting more information; feel free to continue this discussion in chat. $\endgroup$ – thesecretmaster Mar 11 '18 at 13:11
  • $\begingroup$ Early or late returns, or both. Mid-block returns, and returns from loops and nested if statements are smells, and why they should be avoided can be explained in easy terms of hiding the intent (loops should terminate when items are found, deeply nested conditions should be made early checks or the mainline logic) $\endgroup$ – Edwin Buck Mar 15 '18 at 5:42
138
$\begingroup$

Actually returning early should be the norm. Return as soon as you can. There are at least two reasons for this, of which the first, efficiency, is the least important. But if you return early then needless statements won't be executed. Nor will you need to devise some special code to get to the end just so that you can return.

However, the biggest and most important reason for returning early is that the programmer can put a limit on what she/he needs to retain mentally about the state of the computation. This is especially helpful in a sequential if situation. There is actually an Elementary Pattern about this ("Return, not else") that says that if you know what should be returned at the end of an if block, don't use an else, but instead end the block with the return. The reader/writer/updater then gets to ignore the previous state and start over in their thinking process.

If you return only at the end, say "return result;" then you have to track all changes to result that might occur anywhere in the function/method. You have to understand it "as a whole" rather than incrementally. But the reading itself is normally sequential and incremental.

Of course if you always write three statement methods, this won't occur, but few have the discipline to really do that. Sadly, some programmers can't understand their own code when it contains very many nested structures and tests.

I'll also note that this is somewhat related to the idea that we generally try to avoid global variables in programming. Too much extraneous information can impinge on understanding. Being able to comprehend a program "one block at a time" without reference to others, even containing blocks, is a big advantage.


Caveat. Of course, the code should be as clear as possible. If returning late makes it clearer, I wouldn't forbid it. But I think that is seldom the case. Forbidding early returns, however, is counterproductive.

$\endgroup$
  • 4
    $\begingroup$ I think that caveat is more important than calling it a "caveat" may suggest. I'm of the opinion that clear and understandable code is priority #1 (well, maybe #2 after working code), and all other rules can be bent to that aim. But if you do want to have a general principle about early returns, I definitely agree that allowing and even encouraging them seems like a better choice than forbidding them, for exactly the reasons you describe. $\endgroup$ – David Z Mar 9 '18 at 23:16
  • 2
    $\begingroup$ I'd also like to add a caveat -- this answer makes a lot of sense in a high school context, where the language is probably high-level (Python, Java, etc.) with managed memory. In lower-level languages (e.g., C), returning late may make for much cleaner and more readable code. $\endgroup$ – tonysdg Mar 10 '18 at 1:27
  • $\begingroup$ @tonysdg, I don't find the code you link to very compelling. I can make it much cleaner and more understandable with no goto statements at all. For example, within the limits of formatting here, a part of it could be: if (!doB()) {undoB(); undoA(); return;} (undoing both B and A) $\endgroup$ – Buffy Mar 10 '18 at 1:36
  • 1
    $\begingroup$ Modern optimizers are pretty sharp about implementing return early even when you don't do it in your code. The efficiency argument is not as strong as it was when I was learning. $\endgroup$ – pojo-guy Mar 10 '18 at 5:20
  • 1
    $\begingroup$ I was taught never to return early. Then in javascript I realized returning undefined or empty lists early let me scrap all the error cases at the beginning. This cleaned up my code. Then I started using early return cases for edge cases that didn't follow the main flow. Such as if isSpecialCase(input)) { return handleSpecialCase(input); }. Not sure what other uses I will find later, but I think the problem the teacher is trying to address is very disorganized code by beginners which isn't bad intent by any means. $\endgroup$ – aaaaaa Mar 11 '18 at 20:16
48
$\begingroup$

Teach your students about the return early philosophy. Teach your students about the single return philosophy. Tell them this debate is not settled. Explain to them that different software development teams have different opinions and different standards, and that it's important for them to have both these tools in their toolbox for their jobs.

Explain the risks of each:

  • Return early can result in mistakes where the function is modified so that something that should happen unconditionally doesn't or only happens under the wrong conditions.
  • Single return carries the risk that a change that should happen only conditionally might be accidentally made unconditionally or under the wrong conditions.

Both of these risks are greater if the function is particularly long or complex or if the change is a state modification (e.g., database commit). Explain to them that this is a good reason to prefer shorter functions and stateless code when feasible. The important thing is to make the code easier to understand and easier to modify without making mistakes.

Now let them decide when they write their own code. Then give them an assignment to modify their code that carries these risks and let them try it out.

Also use it as a chance to caution them about being dogmatic in software design. Writing software is full of things where there's no clear cut, always right answer. The right choice very often depends on the specific situation and sometimes even the culture of a particular team.

$\endgroup$
  • 13
    $\begingroup$ +1 A function that won't fit in one screen of text is usually too long and needs to be broken up (except when it isn't) $\endgroup$ – pojo-guy Mar 10 '18 at 5:21
  • 6
    $\begingroup$ +1 Students will see both patterns in the future, might as well teach them the merits of both. $\endgroup$ – CaffeineAddiction Mar 10 '18 at 23:47
  • $\begingroup$ It's also important to teach them about coding standards, and that those standards will probably dictate which one they must use. $\endgroup$ – Reinstate Monica Mar 12 '18 at 13:52
  • $\begingroup$ @DoctorJones I wouldn't say "probably," as a lot of shops won't have something like this formally documented. ;) And unless you're working on something that's life and death, a standard to this level of detail is probably too heavy weight. But yes, they should be aware that it's possible. $\endgroup$ – jpmc26 Mar 12 '18 at 15:30
28
$\begingroup$

Is it a wrong approach, giving your students the choice? Or is it wrong to force them never to return early?

This is a false dichotomy. Sometimes multiple returns are clearer, and sometimes (rarely) a single return is clearer. You should encourage ("force" is a rather strong word) your students to use the technique which makes the most sense for any given scenario.

There is one, very specific, situation where "one entry, one exit" makes sense, but it is not something you should expose to beginners, and it is also most likely not what your colleague is talking about. It may still make sense to show at more advanced levels, for students who might (for example) submit pull requests to Linux or Git.

The problem, you see, is C. C does not have any of the following:

  • C++'s destructors and RAII.
  • Java's try-with-resources.
  • Python's with statement and context managers.
  • Go's defer.
  • C#'s using and IDisposable.
  • The finally block, from numerous languages with exceptions including most of the above.

As a result, the organization of cleanup code in C is quite different from higher-level languages. Essentially, the typical way to do cleanup in C is like this:

int read_some_type(char *filename, struct some_type *result){
    int rc = 0;
    if(!result){
        rc = -1;
        goto out;
    }
    int fd = open(filename, O_RDONLY);
    if(fd == -1){
        rc = -1;
        goto out;
    }
    // Normally we'd allocate this on the stack; assume struct some_type is huge.
    char *str = malloc(sizeof(struct some_type));
    if(!str){
        rc = -1;
        goto out_open;
    }
    ssize_t size;
    size_t total = 0;
    while(total != sizeof(struct some_type)){
        size = read(fd, &str[total], total - sizeof(struct some_type));
        if(size <= 0){
            rc = -1;
            goto out_malloc;
        }
        total += size;
    }
    memcpy(result, str, sizeof(struct some_type));
out_malloc:
    free(str);
out_open:
    close(fd);
out:
    // This is strictly unnecessary, but we could add code here in the future.
    // For example, we might check errno and call perror(3) or similar.
    return rc;
}

Needless to say, this is not "using a flag" as your colleague is suggesting. Converting the above example into a flag-based design will result in an excess of if statements, which are too numerous here already. More importantly, it would require the reader to keep track of the flags' states for more than a line or two, which is greatly harmful to readability.

Moving the cleanup into helper functions is more promising but still infeasible, because:

  • You need a helper function for each managed resource, in each main function that needs to manage a resource, plus another helper function for any cleanup code that needs to execute unconditionally.
  • Helpers do not automatically inherit the state of their callers and a lot of state may need to be passed around.
  • The information about which helpers imply which other helpers, and which order the helpers need to be called in, is either repeated throughout the function body and prone to transcription errors, or else the helpers call each other in a chain and must pass around even more state.
  • Reading the cleanup code sequentially at the end of the function is intuitively straightforward. Helpers require human context-switching to properly understand.
  • Functions must have names.
  • Functions invite reuse, which invites tight coupling between the cleanup code of unrelated modules of the system. If there really is some lower-level logical operation which needs to be performed in multiple places, then that might make sense as a helper function.
  • The cleanup blocks are often one-liners, such as close(fd). You cannot meaningfully extract that into a helper, because your helper would just be the close() function with a different name.
  • This function is doing one logical operation (read a struct some_type from a file and extract it into an object in memory). Breaking it apart into several functions means each helper is no longer performing a single logical operation. This is both philosophically and pedagogically objectionable, and also harder to read.

This is one of the few situations in which you truly need to use goto in order to keep code readable and maintainable. Since your students might not otherwise be exposed to that kind of situation, it's worth working through a real example, which you might find in Git, Linux, or any sufficiently large and "interesting" C library or application.

$\endgroup$
  • 2
    $\begingroup$ @Buffy: Expanded and revised. But regardless of your personal opinion, it should be emphasized that Linus won't accept pull requests which use helper functions. So if you plan to submit code to Linux or Git, you must use the goto cleanup method even if you hate it. $\endgroup$ – Kevin Mar 10 '18 at 2:31
  • 4
    $\begingroup$ "but an outer shell can't return until a contained shell does" - Don't you see that the goto statements functionally are returns? $\endgroup$ – Kevin Mar 10 '18 at 3:44
  • 1
    $\begingroup$ I also took the liberty of writing a real example, which I think makes it very clear that helper functions would be much harder to read. $\endgroup$ – Kevin Mar 10 '18 at 4:04
  • 1
    $\begingroup$ @GypsySpellweaver: I do believe I did discuss that, in both the first and last paragraphs. Perhaps if I had not been repeatedly pestered about the viability of this technique, that would have made up a larger component of this answer. Nevertheless, it does appear to answer the question as asked from where I sit. Do you have any specific and actionable suggestions? $\endgroup$ – Kevin Mar 10 '18 at 6:42
  • 4
    $\begingroup$ It is wrong to talk to students about a single vs multi return policy without explaining that it depends on the language. This answer makes clear that single returns are an established c pattern. One that should not be dictated to any of these other languages that have their own construct for cleaning up resources. $\endgroup$ – candied_orange Mar 10 '18 at 21:12
12
$\begingroup$

When I learnt Eiffel, I realised that a single return at the end of a function is the way to go. Eiffel does not have a return. You just set the value of the Result variable, and it is returned at the end of the function. The Eiffel style of programming makes this natural.

When I learnt Scheme, I realised that return early was the way to go. In scheme we created cases, and each case would return a different result. The scheme style of programming makes this natural.

When I program in other languages, I use one or the other, depending on my needs. However I Use one or the other I don't use something in between. Therefore either do a switch-case-return, or return at end. In between is a mess.

For teaching. Remember there is more than one correct way. You will probably want to teach one at a time. And it would be better to choose a language than supports the style that you are teaching. (There are however infinitely more incorrect/bad ways to do it. I am not saying that all ways are good, just that there are more than one).

$\endgroup$
  • $\begingroup$ Some languages make one style prevalent over the other. Some other languages are more flexible and different coding styles can be used. What you do there can be defined either by the style, or the algorithm itself - you do what is cleanest. $\endgroup$ – Baldrickk Mar 14 '18 at 16:26
10
$\begingroup$

Speaking as a computer science professional (not an educator) with over 40 years of experience, the important thing to teach is discipline. Would-be programmers need to understand that code which is poorly structured and disorganized leads to both confusion and "stupid" coding errors.

The criteria you give is a good first cut at a set of "rules" in this area, but it's important to appreciate why the rules are there.

In real life code is maintained, meaning that you or some co-worker may come back months or years later to make some "minor" change. If the person making the change does not, eg, notice that a return is embedded inside an else statement inside a loop then the conditions are set up to introduce a bug. (Similarly, if a bunch of nested if/then statements are coded without associated do/end statements then it's easy to accidentally mess things up.)

It's not the specific rules that is important, it's the purpose behind the rules. As languages (and coding styles) morph and change the specific rules will come and go, but the purpose remains to make the structure of the program visually evident, and to make "stupid" coding errors during program modification less likely.

$\endgroup$
  • 4
    $\begingroup$ Welcome to Computer Science Educators! This is a high quality answer. I think you'll find that you have much to contribute to this site. Perhaps not "in spite" you're not an educator, but because of that. a new perspective is always good. So, welcome, and I hope we'll see more contributions like this one. $\endgroup$ – ItamarG3 Mar 11 '18 at 5:30
3
$\begingroup$

After some reflection on user Kevin's post here, I thought it might be useful to capture some of the comment discussion permanently and also provide to users who don't use C very much (I don't myself lately), a deeper explanation of what is going on.

I agree with Kevin (hereafter the OP = original poster) that this sort of coding isn't what you teach novices first, and is limited, perhaps, to C-like (even assembly-like) languages. Below, I've modified his example just a bit by trying to explicate the various shells that I discussed in a comment. In particular, the only change has been to add a block structure to the code using braces (which can be opened anywhere and create a new lexical level, though I don't need to use if for renaming variables here). It is just to make the structure (I hope) clearer to the reader and the code archaeologist. I've also given "names" to the blocks/levels/shells, though just as comments.

int read_some_type(char *filename, struct some_type *result){
    int rc = 0;
    { //<1> "Code is broken on entry" shell
        if(!result){
            rc = -1;
            goto out;
        }
        { //<2> "Try to open a file" shell
            int fd = open(filename, O_RDONLY);
            if(fd == -1){
                rc = -1;
                goto out;
            }
            { //<3> "Try to allocate" shell
                // Normally we'd allocate this on the stack; assume struct some_type is huge.
                char *str = malloc(sizeof(struct some_type));
                if(!str){
                    rc = -1;
                    goto out_open;
                }
                { //<4> "Evaluate and act" shell
                    ssize_t size;
                    size_t total = 0;
                    while(total != sizeof(struct some_type)){
                        size = read(fd, &str[total], total - sizeof(struct some_type));
                        if(size <= 0){
                            rc = -1;
                            goto out_malloc;
                        }
                        total += size;
                    }
                    memcpy(result, str, sizeof(struct some_type));
                    out_malloc:
                } //</4>
                free(str);
                out_open:
            } //</3> 
            close(fd);
        out:
        } //</2>
    } //</1>
    // This is strictly unnecessary, but we could add code here in the future.
    // For example, we might check errno and call perror(3) or similar.
    return rc;
}

Each "shell" is now a block. Each shell tries to do something that might fail. Each shell ends with a label. Each "goto" is essentially a return from the shell, as was noted by the OP and I also noticed. Each shell is immediately followed by cleanup from that shell. This is a special case, actually, since here the cleanup needs to be done both on success and failure of the overall code. If the cleanup were only needed on failure, the innermost shell could do an immediate return after success is achieved, though goto out here is equivalent as there is no code following that label.

If you think about the code, while it doesn't use "helper" functions, in effect what it is is a bunch of helper functions expanded in place (inlined) rather than called explicitly. The lexical structure replaces arguments. This sort of thing (inlining) is often used for efficiency (and making Torvalds happy, I guess). But in most coding efficiency is the last thing you need to consider (after writing clearer code and then running a profiler to see where efficiency suffers). Kernel code in an OS is an obvious exception, of course.

It is also important to note that each shell cleans up after itself. So both allocation and the needed deallocation take place within the same shell. Similarly for the file handling.

Moreover, if you are going to code this way you need to be absolutely pure in your implementation of it. It is fine to add a new shell, inner to another as long as you keep the same discipline. You don't need to create the extra blocks as I've done here, but you do need to write as if they exist. If you use additional goto statements to jump between the shells/blocks you will have a mess.

Also, if you treat the goto statements as "returns" from the shell, then the idea of "returning as soon as you can" is still in place. Here, you "return" when you notice a failure (say of malloc). But you also "return" at the end of the shell by falling through to the outer shell.


The code is also a bit special since two of the shells (#1 and #2) end at the same place, though that isn't the general situation. In general there might be two different targets for what is, here, the out label. That label would be after the end of the #2 shell at the end of #1


Note this post is "owned" by the community so you are free to improve it as well as comment on it. In particular, I'd be interested in hearing from the OP whether making these shells/levels explicit would be considered to be a good or bad thing in his environment.

$\endgroup$
  • 1
    $\begingroup$ Personally, I find the extra indents much harder to read (consult also the kernel style guide on indentation, which de facto bans this because there are too many levels of indentation). But I do agree that this might be pedagogically useful, to help students understand what the gotos are for and why we use them. I just can't imagine seeing it "in the wild." $\endgroup$ – Kevin Mar 11 '18 at 17:16
2
$\begingroup$

This is a great opportunity to teach about history of programming and how programming itself evolves over time. Keep in mind that "evolve" is neither a synonym nor an antonym to "improve"; it's an independent word with its own meaning.

20 years ago Single Entry Single Exit was a somewhat popular coding style, but that time has passed. Nowadays the popular style is short functions, early returns, and absence of deep nesting. Source: (because I'm lazy) The answers on this question and their respective vote count.

The above certainly applies to C++ and C#. Popular coding styles can and will differ between different languages - sometimes for practical reasons, sometimes for historical reasons. They will also differ among different companies and different teachers.


To actually answer your question: Teach them what coding styles are acceptable in your class, and why. It doesn't matter which style(s) you chose, but you must answer the why. Also teach them that other coding styles exist, many of which they will encounter in other courses and in their professional career.

$\endgroup$
1
$\begingroup$

I propose that the correct answer is to keep doing what you're doing, by teaching the techniques that you're currently teaching. Both of you.

I was taught with the approach you use. When I was first exposed to the other approach, I didn't much like it. But with an open mind, I learned that the approach used by the other instructor is useful for some analysis processes. Most especially this refers to some particularly older "source code analysis" software tools, but there may be some ongoing benefit to that other style too. As I didn't have experience with the practice of using those specific auditing processes, I'm not in the best position to judge whether the benefits outweigh the pains of constraints.

So, have students be taught both methods.

Maybe students won't like being coerced into being graded on how well they use one method, only to be later graded in another class based on how well they accomplish coding using the other style. The transition may be unpleasant. Tough. That's great.

It is good for students to be prepared for the workforce, and many may benefit greatly from learning that an expected style might not match whatever their personal preference is. If they are allowed to get locked into one style, that may cause challenges in the future when they transition into their first employer, or transition between companies, or even transitioning between different departments of an organization that may have different policies for different departments. Getting the students uncomfortable if they don't adapt may be better for them to experience now.

If having different requirements leads to them being uncomfortable, tell them that different employers may expect different things, and even different projects (e.g., different open source projects) that they may want to become a part of during their off-time. So may different instructors. They are welcome to use whatever approach they want for their own personal hobby projects that they are in charge of, but when other people are in charge, then cooperation can be oodles more important than advantages of a particular style.

So, no, my advice here is: don't give them a choice. Make 'em adapt.

$\endgroup$
  • $\begingroup$ I am reminded of One's Final Words from ST:Voy (YouTube Video) Though hopefully not at all for the same reason, if the student gives the same response as what we see from Seven of Nine Tertiary Adjunct of Unimatrix Zero One over the next ten seconds, that effect just might be precisely the best that you can hope for... $\endgroup$ – TOOGAM Mar 11 '18 at 2:38

protected by thesecretmaster Mar 11 '18 at 13:14

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

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