Don't ALWAYS quick-return from functions

Ben Winding on April 13, 2018

There are two main styles of returning from a function in a program. Both styles are almost diametrically-opposed, but have their uses. ... [Read Full]
markdown guide
 

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.

 

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!

 

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!

 

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

 

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

 

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++...

 

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 :)

 

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.

 

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

 

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

 
 

Single returns is always better unless 0.0000001% cases when you are writing in JS and found a weird bug of V8 JIT.

 

Your final code snippet seems to be missing some braces

 
 

Yes. But I should point out that your final code snippet and the initial one may not do the same thing.

Sppose the request is not Important, has no discount, and is not a member. The initial GetCost() will return 20; the final one will return 12.

The only way for the two to be exactly identical in function is if certain conditions are mutually exclusive, but that isn't stated anywhere.

Ahh I see, cheers for that. I'm beginning to regret using the embarrassing technique of using the audience as the debugger.

code of conduct - report abuse