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;
}

Answer 1

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.

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

Answer 3

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