Ben Winding

Posted on Apr 13, 2018 • Edited on Apr 15, 2018 • Originally published at blog.benwinding.com

Don't ALWAYS quick-return from functions

#functions #refactoring #cleancode #quickreturn

There are two main styles of returning from a function in a program. Both styles are almost diametrically-opposed, but have their uses.

Single Return

An example of nested single-returning. This is hard to read and keep track of the levels of scope.

int GetCost(request) {
    int cost = 0;
    if (request.Important) {
        if (request.HasDiscount) {
            if (request.IsMember) {
                cost = 10;
            }
            else {
                cost = 12;
            }
        }
        else {
            cost = 14;
        }
    }
    else {
        cost = 20;
    }
    return cost;
}

Quick Return

Now arguably the code can be made simpler by using the prophetic "Quick Return!" method. This has the effect of returning out of the function as soon as possible, making the code easier to reason about. Also, by unravelling the if-statements into a neat stack, the code becomes much easier to read.

int GetCost(request) {
    if (!request.Important) 
        return 20;
    else if (!request.HasDiscount) 
        return 14;
    else if (!request.IsMember) 
        return 12;
    else
        return 10;
}

However ... Memory Leaks

The reason quick returns were not encouraged in earlier days of programming is a lack of garbage collection. During the execution of a function, memory may be allocated. So a simplify this, the function is given only one exit point, to ensure all memory in the function is deallocated before the function returns.

The following is another version of the single return function (with an added checking object). Cleaner and easier to read with still one return statement (and a little clean up).

int GetCost(request) {
    Checker checker = new Checker(request);
    int cost = 0;
    if (!checker.Important) 
        cost = 20;
    else if (!checker.HasDiscount) 
        cost = 14;
    else if (!checker.IsMember) 
        cost = 12;
    else 
        cost = 10;
    delete checker;
    return cost;
}

But memory might not be the only thing that needs to be cleaned up at the end of a function's execution. There are other things that might need to be 'freed up' like:

Hardware resources
Object mutation locks
Network connections

By returning only once within a function, the chance of leaving one of these resources open/locked is greatly reduced.

So, the older method of single return is not completely useless! And depending on the language and environment, quick-returns may not always be the best solution.

Top comments (16)

Idan Arye • Apr 14 '18

RAII - if the language has it - is usually the best way to free resources at the end of a function. But if you have to free them manually, many modern languages have a try...finally construct:

int GetCost(request) {
    Checker checker = new Checker(request);
    try {
        int cost = 0;
        if (!checker.Important) 
            return 20;
        if (!checker.HasDiscount) 
            return 14;
        if (!checker.IsMember) 
            return 12;
        else
            return 10;
    } finally {
        delete checker;
    }
}

This will guarantee that no matter where which return statement ends the function, and even if there was an exception somewhere - delete checker will be invoked.

Andrew Buntine • Apr 13 '18

Nice writeup. In general I am not a fan of multiple returns per function (especially for very simple functions), although I am not sure I am following your argument.

In order for a function to leak memory in this fashion you'd need to be working in a language that lacks a garbage collector. So let's say the above is C or C++. If that's the case, the "request" struct would/should always be passed by reference (a pointer) and not by value. I am sure there are some situations where this type of single return is favourable due to potential memory leaks - but I'm just not sure this example is one of them. :)

I also think perhaps there is a bug in the solution as you always assign "cost=10" before returning it!

Ben Winding • Apr 14 '18

Thanks for the feedback Andrew! I've fixed the last example code in order to better demonstrate what I meant.

Yes, the argument I was trying to make is mainly concerned, but not limited to garbage collection. Actually, any resource may need to be 'freed' at the end of the function; boolean locks, system resources etc...

Also fixed the bug you spotted, thanks!

Andrew Buntine • Apr 14 '18

Yeah, nice one. The last example demonstrates the concept much better now.

Jan van Brügge • Apr 15 '18

The code is not really a good example as you shouldn't use new any more due to the problems outlined. Your code can be written this way:

int GetCost(request) {
    Checker checker = Checker(request);
    int cost = 0;
    if (!checker.Important)
        return 20;
    if (!checker.HasDiscount)
        return 14;
    if (!checker.IsMember)
        return 12;

    return 10;
}

If you want to use pointers, you should always wrap them in a smart pointer, e.g. std::make_shared(new Checker). That way they will also be removed when the scope is exited

Ben Winding • Apr 15 '18

Thanks for the feedback,

Smart pointers make me feel dumb, but they look like a great tool. I have a lot to learn about modern c++...

MTrost • Apr 15 '18 • Edited

In your last example, shouldn't these be else if-statements when you check hasDiscount and isMember?

Because as it now stands it always goes into the else-statement if the request is ~~not ~~a member, therefore always returning 10, even if it is important or has a discount.~~~~

But it's late for me so perhaps I don't get it :D

Anywho: Great write-up anyway, thanks for sharing!

EDIT: just realized the ! before the checks, so ignore my tired rambling :)

fabulous.yap • Apr 15 '18

nice write-up. am guilty of always refactor codes to return only once, it makes the codes cleaner and easier to debug... tho not doing it mainly for freeing of resources, as imho it's better handled with other constructs (try...finally, smart pointers, etc.) if the language supports it...

the last refactored seems to have a small logic issue, assuming the formatting is based on C/C++ language, hope you don't mind that I copy your code to create a small test program here... tpcg.io/JbGrUA

Ben Winding • Apr 15 '18

Thanks man, I don't mind at all, wow that's pretty cool you could do that online

inf3rno • Apr 15 '18 • Edited

One of the advantages of functional programming, that you can write a wrapper, which locks and unlocks the resources automatically, so you won't forget anything to unlock. This approach has it's limitations too ofc. Nice article though, I did not know that was the cause.