Another example of premature optimisation

G'day:
I initially posted this to Stack Overflow, but with a mind to releasing it here too. It's worth repeating.

The original question was along these lines. Given two approaches to making two decisions, which is the most performant:

function f(){

    // other code redacted

    if (firstBooleanValue && secondBooleanValue){
        return true;
    }
    return false;
}

Or:

function f(){

    // other code redacted

    if (firstBooleanValue){
        if (secondBooleanValue){
            return true;
        }
    }
    return false;
}

Someone else answered with a loop-de-loop amplification test to demonstrate [something, I don't care which way it came out, and nor should you], but they did caveat it with "it doesn't matter".

My answer was as follows:

I know @tom starts his answer by saying "it doesn't matter", but this cannot be stressed enough, so I'm going to elaborate further in a separate answer.

@piotr-gajdowski: just don't worry about this sort of thing. If one needs to loop thousands of times to see a difference, this implies it's a case of micro-optimisation. The differences here will be lost in the background noise of any other consideration going on at the same time, for example:

• what else the server is doing at the time;
• network latency variation;
• how busy external dependencies like DBs are at the time.

So... it simply doesn't matter.

Always go for the code that you find clearest, and - more importantly - that other people will find the clearest. This generally means the simplest.

For your specific situation, given you given us "fake" code, it's impossible even to say given your example what that might be:

• how complex are the two conditions?
• Are they really just single variables?
• Do the variables have explanatory names?
• If they conditions are actually boolean expressions rather than variables... consider putting them in well-named intermediary variables.

These are the things you should be worried about.

Write your code as clearly as possible (this doesn't not necessarily mean as tersely as possible, btw). If at some point you find you have performance issues, then (and only then) start looking for optimisations. But don't start looking for this sort of optimisation. Start looking for things that will make a difference.

I think the best thing you could do with your code example would be to submit the real world code to Code Review, and then we can help you come up with the clearest approach to coding it. That's the question you should be asking here.

If all we had to go on was the example provided, then it's difficult to say what the clearest approach would be here. The variable names are nonsense in the posted code, so one cannot infer much from them.

If say we are talking about two variables, and they're clearly named, this would be a clear option:

function canAttendMeeting(){

    // other code redacted

    return hasFreeTime && isInOffice;
}

If however there's no intermediary descriptive variables hasFreeTime and isInOffice, then don't do this:

function canAttendMeeting(){

    // other code redacted

    return (meetingStart > dayBegin & meetingStart < datEnd & withinFreePeriod(meetingStart, meetingEnd)) && (person.locationAtTime(meetingStart, meetingEnd) == "office");
}

Stick 'em in intermediary variables.

If the logic to derive the second boolean expression is somehow predicated on the first condition condition being true, and short circuiting won't work, then I'd consider taking an early exit over a nested if:

function f(){

    // other code redacted

    if (not firstBooleanValue){
        return false;
    }
    if (secondBooleanValue){
        return true;
    }
    return false;
}

All things being equal, I attempt to keep my code as close to the left margin as possible. If I have to scan for the end of a logic block, rather than just see it at the same time as the top of the logic block, I've probably got a chance to simplify the logic. I am always a bit wary of of having a negative expression in a decision statement, so if poss I'd try to reverse the name of an intermediary variable:

function canAttendMeeting(){

    // other code redacted

    if (doesNotHaveFreeTime) {
        return false;
    }

    // other code only possible if the person has free time
    
    return isInOffice;
}

And there will be other variations which are more or less viable given the circumstances. But generally the consideration is code clarity, not micro-optimising a situation not yet known to need it. And in that case, one needs to consider things a bit more clearly than how to order if statements.

Righto.

--
Adam