CODEWITHSUNDEEP
c
Adam S
Adam S

Reputation: 9235

Readable conditional logic without unnecessary execution?

I'm trying to make the below code both readable and performant. I want to avoid any unnecessary call to getFlagB() while also not repeating anything. Below I have written two methods, each which satisfies exactly one of these criteria.

Assume getFlagB() cannot be altered in any way. Is there a way to meet both these requirements simultaneously in C, without creating additional flags?

// Method 1 - doesn't repeat code blocks but calls getFlagB even when it may not need to
void foo(int flagA)
{
    int flagB;
    getFlagB(&flagB);

    if(flagA & flagB)
    {
        // Code block 0
    }
    else
    {
        // Code block 1
    }
}

// Method 2 - doesn't no extra call to getFlagB, but repeats code block 1
void foo(int flagA)
{
    int flagB;

    if(flagA)
    {
        getFlagB(&flagB);
        if(flagB)
        {
            // Code block 0
        }
        else
        {
            // Code block 1
        }
    }
    else
    {
        // Code block 1
    }
}

Upvotes: 5

Views: 154

Answers (6)

John Vulconshinz
John Vulconshinz

Reputation: 1148

I cannot definitively say anything because I have not seen any actual code but it seems to me the flagA is irrelevant and can be ignored. While flagB must be evaluated because it is relevant and causes the code to change.

void foo()
{
    getFlagB(&flagB)
    if(flagB)
    {
        //Code 1
    }
    else
    {
         //Code 0
    }
}

But I am assuming that you do not have an unnecessary flag in your program. So I would recommend doing the seconded one it is more efficient and elegant even thought it does not seem that way.

Upvotes: 1

Potatoswatter
Potatoswatter

Reputation: 137820

Compute the condition explicitly before the if.

_Bool condition = flagA;

if ( flagA ) { /* First decide what to do. */
    _Bool flagB;
    GetFlagB( & flagB );

    condition = flagA && flagB; /* Prefer && over &. */
}

if ( condition ) { /* Then do it. */
    Code1();
} else {
    Code2();
}

Upvotes: 1

user539810
user539810

Reputation:

EDIT

You can also do the following, so long as flagB is initialized to 0. This avoids a bug in the code I posted earlier that assumes the flags aren't modified inside code block 0, which can cause both code blocks 0 and 1 to execute. I'd recommend this new one only because you may at some point want to modify the flags inside foo():

int flagB = 0;

if (flagA)
    getFlagB(&flagB);

if (flagA && flagB) {
    // code block 0
} else {
    // code block 1
}

Yes, flagA is tested twice. You have a ternary condition, but you're asking for a binary set of outcomes. Without using sequence points as one person mentioned, you must test twice or duplicate code or unnecessarily call a function or add extra function call overhead if flagA happens to be set.

They're all valid solutions IMHO. It is just a matter of how readable and how well performing you want the code to be, not to mention avoiding code duplication... Nobody likes that! :-)

Happy coding!

Upvotes: 1

John3136
John3136

Reputation: 29266

Wrap getFlagB() in another method, then let the compiler sort it out for you.

int myGetFlagB() { int b; getFlagB(&b); return b; }

void foo(int flagA)
{
    /* note: assume you mean && and not &, otherwise there is no way
     * to short circuit - you always need to call getFlagB for a
     * bitwise AND.
     */
    if(flagA && myGetFlagB())
    {
        // Code block 0
    }
    else
    {
        // Code block 1
    }
}

Upvotes: 2

cmaster - reinstate monica
cmaster - reinstate monica

Reputation: 40625

If you really do not want to either encapsulate the getFlagB call or split your if, you can abuse the comma operator:

if(flagA && (getFlagB(&flagB), flagB)) {

Even though it does not look nice, it does precisely what you want.

Upvotes: 1

nouney
nouney

Reputation: 4411

You can do this:

void foo(int flagA)
{
    int flagB;

    if(flagA)
    {
        getFlagB(&flagB);
        if(flagB)
        {
            // Code block 0
            return ;
        }
    }
  // code block 1
}

Upvotes: 3

Related Questions