If statement not calling all methods returning BOOL

Question

I have code like this:

-(IBAction)send {        
    if ([self isCorrect1] && [self isCorrect2] && ...) {
        [self sendRequest];
    }
}

-(BOOL)isCorrect1 {
     ...
}

-(BOOL)isCorrect2 {
     ...
}

Every isCorrect method is checking some condition showing some message in the view and returning result of the checking. I noticed that if first condition is false it will only show error message for the first method (and I need all of them to be checked) and no breakpoint is triggered inside these methods. I thought it was some kind of LLVM optimization so I created code like this:

-(IBAction)send {
    BOOL correct = [self isCorrect1];
    correct = correct && [self isCorrect2];
    ...

    if (correct) {
        [self sendRequest];
    }
}

And is still not working correctly. Do I have to create new BOOL variable to store result for the check or is there some other way?

Sure it doesn't. The logical or operator short-circuits. There are hundreds of dupes of this question already. — , Oct 11 '13 at 11:00
In this case ( all && ) if statement will stop checking after first NO — Dobroćudni Tapir, Oct 11 '13 at 11:00
possible duplicate of [Is short-circuiting boolean operators mandated in C/C++? And evaluation order?](http://stackoverflow.com/questions/628526/is-short-circuiting-boolean-operators-mandated-in-c-c-and-evaluation-order) — , Oct 11 '13 at 11:01
Ok but what with `correct = correct &&` approach. Why it's short-cutting? — Tomasz Bąk, Oct 11 '13 at 11:05

blackzero · Accepted Answer · 2013-10-11T11:26:37.493

3

Since the first condition is evaluated to false, it won't check for the rest of the conditions and will go to the else part straightaway.

Try this.

BOOL finalResult = [self isCorrect1];
finalResult = [self isCorrect2] && finalResult;
finalResult = [self isCorrect3] && finalResult;
finalResult = [self isCorrect4] && finalResult;
...

if (finalResult) {

}

This will go through all of the isCorrect tests and will let you know if it passed all of them in the end or not.

edited Oct 11 '13 at 11:26

answered Oct 11 '13 at 11:07

blackzero

78
7

This has the same short-circuiting behaviour as the OP's original code (namely, evaluation will stop at the first failing condition, and remaining conditions are not evaluated) – Dirk Oct 11 '13 at 11:23
Changed order of the method and the flag will stop compiler from short-cutting? – Tomasz Bąk Oct 11 '13 at 11:33
Checked - it works with LLVM 5.0 and it's more clean than other solutions. Thanks. – Tomasz Bąk Oct 11 '13 at 11:42

Dirk · Answer 2 · 2013-10-11T11:17:21.617

The behaviour you see is the expected behaviour of the &&, namely, it "short-circuits" the evaluation, if it can determine the result in advance, before having evaluated all conditions:

expression-yielding-false && something-else

The result of the above is completely determined by the first part; regardless of what the second operand yields, the final result is false. This allows you to write something like:

if (obj != null && obj->count == 3) 
{
    ...
}

If the && did not have the short-circuit behaviour, you'd have to write

if (obj != null) 
{
    if (obj->count == 3)
    {
       ...
    }
}

The || has a similar behaviour. In case of

something-yielding-true || anything

the right-hand side cannot affect the result value, as the left-hand side already returned true.

One possible work-around would be:

int succeeses = 0;

succeesses += [self isCorrect1]? 1 : 0;
succeesses += [self isCorrect2]? 1 : 0;
succeesses += [self isCorrect3]? 1 : 0;

if (successes == 3) 
{
    // All tests did succeed
}
else
{
    // At least one failed.
}

If you need to know, which tests passed, and which failed, you can try:

BOOL passed1 = [self isCorrect1];
BOOL passed2 = [self isCorrect2];
BOOL passed3 = [self isCorrect3];

if (passed1 && passed2 && passed3) 
{
    // All tests did succeed
}
else
{
    // At least one failed.
}

A more dense version of the above would be

int passed = 0;

passed |= [self isCorrect1]? (1 << 0) : 0;
passed |= [self isCorrect2]? (1 << 1) : 0;
passed |= [self isCorrect3]? (1 << 2) : 0;

if (passed == 7) 
{
    // All tests did succeed
}
else
{
    if (passed & (1 << 0)) 
    {
        // First test passed
    }
    else 
    {
        // First test failed
    }
    if (passed & (1 << 1)) 
    {
        // Second test passed
    }
    else 
    {
        // Second test failed
    }
    if (passed & (1 << 2)) 
    {
        // Third test passed
    }
    else 
    {
        // Third test failed
    }
}

which is simply a more occult formulation of the version with a boolean variable per test tried.

Thank you for examples. Second one worked for me. I would argue that third approach is an overkill for readability. — Tomasz Bąk, Oct 11 '13 at 11:30
I tend to agree. The only advantage is, that you can evaluate, which conditions failed, using a `switch` statement, too, which is helpful sometimes. — Dirk, Oct 11 '13 at 11:42

If statement not calling all methods returning BOOL

2 Answers2