DEV Community

Cover image for Don't ALWAYS quick-return from functions

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. ...
Collapse
 
idanarye profile image
Idan Arye

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.

Collapse
 
buntine profile image
Andrew Buntine

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!

Collapse
 
benwinding profile image
Ben Winding

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!

Collapse
 
buntine profile image
Andrew Buntine

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

Collapse
 
jvanbruegge profile image
Jan van Brügge

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

Collapse
 
benwinding profile image
Ben Winding

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

Collapse
 
mtrost profile image
MTrost • 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 :)

Collapse
 
fabulousyap profile image
fabulous.yap

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

Collapse
 
benwinding profile image
Ben Winding

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

Collapse
 
inf3rno profile image
inf3rno • 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.

Collapse
 
msoedov profile image
Alex Miasoiedov

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

Collapse
 
k2t0f12d profile image
Bryan Baldwin

Don't use new. That's your problem.

Collapse
 
shalvah profile image
Shalvah

Your final code snippet seems to be missing some braces

Collapse
 
benwinding profile image
Ben Winding

Cheers, is that fixed now?

Collapse
 
shalvah profile image
Shalvah • Edited

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.

Thread Thread
 
benwinding profile image
Ben Winding

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