Efficiency over Do-While(false) block vs return

Question

So the question is which of these implementation has better performance and readability.

Imagine you have to write a code that each step is dependent of the success of the previous one, something like:

bool function()
{
    bool isOk = false;

    if( A.Func1() )
    {
        B.Func1();

        if( C.Func2() )
        {
            if( D.Func3() )
            {
             ...
             isOk = true;
            }
        }
    }

    return isOk;
}

Let's say there are up to 6 nested IFs, since I don't want the padding to grow too much to the right, and I don't want to nest the function calls because there are several parameters involved, the first approach would be using the inverse logic:

bool function()
{

    if( ! A.Func1() ) return false:

    B.Func1();

    if( ! C.Func2() ) return false;

    if( ! D.Func3() ) return false;
    ...

    return true;
}

But what about avoiding so many returns, like this:

bool function()
{
    bool isOk = false;

    do
    {
        if( ! A.Func1() ) break:

        B.Func1();

        if( ! C.Func2() ) break;

        if( ! D.Func3() ) break;
        ...
        isOk = true;
        break;

    }while(false);

    return isOk;
}

This has been discussed before, several years ago. Finding the discussion may take some doing, but it got moderately highly voted (and may have been in C rather than C++, but the issues are rather similar). — Jonathan Leffler, Apr 04 '14 at 23:53
Also, is there a problem with an appropriately laid out `if (A.func1() && B.func1() && C.func2() && D.func3()) return true; else return false;`? Or even: `return A.func1() && B.func1() && C.func2() && D.func3();`? — Jonathan Leffler, Apr 04 '14 at 23:56
I admit to having used the `do..while(0);` version once or twice, it's always a last resort though... if possible, I prefer to use a function that returns. — M.M, Apr 05 '14 at 00:23
Like so many such "style" questions, I don't think there's a **right** answer. If the problem fits the middle solution, I'd use that. However, I've often used the `do..while(0)` when appropriate: for instance, if you want to do slightly more than `return isOk` at the end of the sequence of function calls, and creating a new function just so you can return `false` at each stage seems overkill. — TripeHound, Apr 22 '14 at 16:00

score 4 · Accepted Answer · edited May 23 '17 at 10:26

Compilers will break down your code to simple instructions, using branch instructions to form loops, if/else etc, and it's unlikely your code will be any different at all once the compiler has gone over it.

Write the code that you think makes most sense for the solution you require.

If I were to "vote" for one of the three variants, I'd say my code is mostly variant 2. However, I don't follow it religiously. If it makes more sense (from a "how you think about it" perspective) to write in variant 1, then I will do that.

I don't think I've ever written, or even seen code written like variant 3 - I'm sure it happens, but if your goal is to have a single return, then I'd say variant 1 is the clearer and more obvious choice. Variant 3 is really just a "goto by another name" (see my most rewarded answer [and that's after I had something like 80 down-votes for suggesting goto as a solution]). I personally don't see variant 3 as any better than the other two, and unless the function is short enough to see do and the while on the same page, you also don't actually know that it won't loop without scrolling around - which is really not a good thing.

If you then, after profiling the code, discover a particular function is taking more time than you think is "right", study the assembly code.

Just to illustrate this, I will take your code and compile all three examples with g++ and clang++, and show the resulting code. It will probably take a few minutes because I have to actually make it compileable first.

Your source, after some massaging to make it compile as a singe source file:

class X
{
public:
    bool Func1();
    bool Func2();
    bool Func3();
};

X A, B, C, D;

bool function()
{
    bool isOk = false;

    if( A.Func1() )
    {
        B.Func1();

        if( C.Func2() )
        {
            if( D.Func3() )
            {
                isOk = true;
            }
        }
    }

    return isOk;
}


bool function2()
{

    if( ! A.Func1() ) return false;

    B.Func1();

    if( ! C.Func2() ) return false;

    if( ! D.Func3() ) return false;

    return true;
}


bool function3()
{
    bool isOk = false;

    do
    {
        if( ! A.Func1() ) break;

        B.Func1();

        if( ! C.Func2() ) break;

        if( ! D.Func3() ) break;

        isOk = true;
    }while(false);

    return isOk;
}

Code generated by clang 3.5 (compiled from sources a few days ago):

_Z8functionv:                           # @_Z8functionv
    pushq   %rax
    movl    $A, %edi
    callq   _ZN1X5Func1Ev
    testb   %al, %al
    je  .LBB0_2

    movl    $B, %edi
    callq   _ZN1X5Func1Ev
    movl    $C, %edi
    callq   _ZN1X5Func2Ev
    testb   %al, %al
    je  .LBB0_2

    movl    $D, %edi
    popq    %rax
    jmp _ZN1X5Func3Ev           # TAILCALL

    xorl    %eax, %eax
    popq    %rdx
    retq


_Z9function2v:                          # @_Z9function2v
    pushq   %rax
    movl    $A, %edi
    callq   _ZN1X5Func1Ev
    testb   %al, %al
    je  .LBB1_1

    movl    $B, %edi
    callq   _ZN1X5Func1Ev
    movl    $C, %edi
    callq   _ZN1X5Func2Ev
    testb   %al, %al
    je  .LBB1_3
    movl    $D, %edi
    callq   _ZN1X5Func3Ev
                                        # kill: AL<def> AL<kill> EAX<def>
    jmp .LBB1_5
.LBB1_1:
    xorl    %eax, %eax
    jmp .LBB1_5
.LBB1_3:
    xorl    %eax, %eax
.LBB1_5:
                                        # kill: AL<def> AL<kill> EAX<kill>
    popq    %rdx
    retq

_Z9function3v:                          # @_Z9function3v
    pushq   %rax
.Ltmp4:
    .cfi_def_cfa_offset 16
    movl    $A, %edi
    callq   _ZN1X5Func1Ev
    testb   %al, %al
    je  .LBB2_2

    movl    $B, %edi
    callq   _ZN1X5Func1Ev
    movl    $C, %edi
    callq   _ZN1X5Func2Ev
    testb   %al, %al
    je  .LBB2_2

    movl    $D, %edi
    popq    %rax
    jmp _ZN1X5Func3Ev           # TAILCALL
.LBB2_2:
    xorl    %eax, %eax
    popq    %rdx
    retq

In the clang++ code, the second function is very marginally worse due to an extra jump that one would have hoped the compiler could sort out being the same as one of the others. But I doubt any realistic code where func1 and func2 and func3 actually does anything meaningful will show any measurable difference.

And g++ 4.8.2:

_Z8functionv:
    subq    $8, %rsp
    movl    $A, %edi
    call    _ZN1X5Func1Ev
    testb   %al, %al
    jne .L10
.L3:
    xorl    %eax, %eax
    addq    $8, %rsp
    ret
.L10:
    movl    $B, %edi
    call    _ZN1X5Func1Ev
    movl    $C, %edi
    call    _ZN1X5Func2Ev
    testb   %al, %al
    je  .L3
    movl    $D, %edi
    addq    $8, %rsp
    jmp _ZN1X5Func3Ev

_Z9function2v:
    subq    $8, %rsp
    movl    $A, %edi
    call    _ZN1X5Func1Ev
    testb   %al, %al
    jne .L19
.L13:
    xorl    %eax, %eax
    addq    $8, %rsp
    ret
.L19:
    movl    $B, %edi
    call    _ZN1X5Func1Ev
    movl    $C, %edi
    call    _ZN1X5Func2Ev
    testb   %al, %al
    je  .L13
    movl    $D, %edi
    addq    $8, %rsp
    jmp _ZN1X5Func3Ev

_Z9function3v:
.LFB2:
    subq    $8, %rsp
    movl    $A, %edi
    call    _ZN1X5Func1Ev
    testb   %al, %al
    jne .L28
.L22:
    xorl    %eax, %eax
    addq    $8, %rsp
    ret
.L28:
    movl    $B, %edi
    call    _ZN1X5Func1Ev
    movl    $C, %edi
    call    _ZN1X5Func2Ev
    testb   %al, %al
    je  .L22
    movl    $D, %edi
    addq    $8, %rsp
    jmp _ZN1X5Func3Ev

I challenge you to spot the difference aside from the label names between the different functions.

Exactly. Proponents of the single-return style always overlook that (a) the compiler always compiles to single-return code anyway, and therefore (b) both forms are formally equivalent, as the compiler can implement a formal transformation, so (c) there is no formal reason to prefer one over the other. — user207421, Apr 04 '14 at 23:42
Single return has it's (minor) benefits - for example being able to set a breakpoint at the return line and know that you always end up there (without resorting to assembler code inspection). — Mats Petersson, Apr 04 '14 at 23:52
@Mats: Just do an execute until return. Also, some debuggers allow a breakpoint on the closing brace of the function. — Deduplicator, Apr 04 '14 at 23:56
I may of course not be IN the function that I want to check the return value of, just know that eventually it will get there. Setting a breakpoint on the closing brace sometimes works. But overall, I think writing code that is READABLE is more important than how many returns you have, or what "tricks" you use to avoid having many returns, etc. — Mats Petersson, Apr 04 '14 at 23:59

score 2 · Answer 2 · answered Apr 04 '14 at 23:40

2

I think performance (and most likely even binary code) will be the same with any modern compiler.

Readability is somewhat a matter of conventions and habits.

I personally would prefer the first form, and probably you would need a new function to group some of the conditions (I think some of them can be grouped together in some meaningful way). The third form looks most cryptic to me.

answered Apr 04 '14 at 23:40

nullptr

11,008
1
23
18

I'm using the third form quite often in test functions. – wonko realtime Apr 04 '14 at 23:51

score 1 · Answer 3 · answered Apr 04 '14 at 23:35

1

As C++ has RAII and automatic cleanups, I tend to prefer the bail-out-with-return-as-soon-as-possible solution (your second one), because the code gets much cleaner IMHO. Obviously, it's a matter of opinion, taste and YMMV...

answered Apr 04 '14 at 23:35

Massa

8,647
2
25
26

Me too, though sometimes I allow a few levels of conditionals to accumulate, as long as things become more compact. – Deduplicator Apr 04 '14 at 23:53

Efficiency over Do-While(false) block vs return

3 Answers3