CODEWITHSUNDEEP
javaoptimizationif-statement
jadkik94
jadkik94

Reputation: 7078

What is the most optimized way to use if statements in a loop?

I have a list I need to process. The items are either enabled or disabled. The user can choose whether or not to show disabled items.

So you have cond2 that depends on the items, and cond1 that does not. Here's the dilemma I got into: Should I use cond1 && !cond2 or !(!cond1 || cond2)? Or should I check for the cond2 (show disabled items) before the loop? I also thought (as you will see in the code I put) if I should put the cond2 before cond1, because cond2 is a boolean variable, and with "short-circuits" (lazy evaluation?), it will be faster?

My main concern was speed. If I have many items in the loop, this might be an important change.

This is code that illustrates the options:

// First Option
for (String item : items) {
    doSomethingFirst(item);
    if (isDisabled(item) && !showDisabled) {
        continue;
    }
    doSomethingElse(item);
}

// Second Option
for (String item : items) {
    doSomethingFirst(item);
    if (!(!isDisabled(item) || showDisabled)) {
        continue;
    }
    doSomethingElse(item);
}

// Third Option
if (showDisabled) {
    for (String item : items) {
        doSomethingFirst(item);
        doSomethingElse(item);
    }
} else {
    for (String item : items) {
        doSomethingFirst(item);
        if (isDisabled(item)) {
            continue;
        }
        doSomethingElse(item);
    }
}

So, does the order of isDisabled(item) and showDisabled matter? Should I be checking on things before the loop? Or does the compiler optimize that? (I doubt...)

PS I don't know how I would take measurements to see actual values, if it's relevant please do.

Thanks.

Upvotes: 0

Views: 3308

Answers (5)

Michael Buen
Michael Buen

Reputation: 39403

Regarding readability, another good practice is to name functions and variables in their positive state. It's hard to read double negatives. Which one would you rather read?

enabled

Or

!disabled

There are some few cases though that naming things in their negative form makes sense. An example, if you will use them for terminating condition, e.g. end of file. while(!feof(fp))

But in most cases, the norm should be is to name things in their positive form, so reading code has lesser friction.

Let's see how your code looks like in positive form:

// First Option
for (String item : items) {
    doSomethingFirst(item);

    // if (isDisabled(item) && !showDisabled) {

    if (!isEnabled(item) && showEnabled) {
        continue;
    }
    doSomethingElse(item);
}

The readability on that code surely improved.

And even the following become readable too, the double negatives can be avoided, reading code is really merely reading it, not figuring it out too much. I once read an article that suggest that when writing code, it should be very pleasant to read too, he said reading code should not be like reading a detective novel. Reading double negatives code is like reading and deciphering a detective novel.

// Second Option
for (String item : items) {
    doSomethingFirst(item);

    // if (!(!isDisabled(item) || showDisabled)) {

    // You can now avoid double negatives
    if (!( isEnabled(item) || !showEnabled )) {
        continue;
    }
    doSomethingElse(item);
}

In fact, the following is not merely double negative, it's a triple one:

if (!(!isDisabled(item)

  1. isDisabled
  2. !isDisabled
  3. !(!isDisabled

It will take a two-pass read, or even a three-pass before you can decipher what's the intent of that code

Upvotes: 2

alain.janinm
alain.janinm

Reputation: 20065

In Java the expression is evaluated from left to right. With && if the first condition is false, the second one is not executed, with || if the first condition is true, the second one is not executed.

So try to put the condition that can be resolve faster in first place, in your case showDisabled.

For the third example, it looks better because you check the boolean only once but I guess it don't really change performance, a bool comparison is not really costly. You will probably have better improvement to do in other part of your code. (and for the readable aspect it's not my favorite - quite long)

If you want to measure the performance in your case use a profiler for example.

Or add in your code :

long start=System.currentTimeMillis();
//code to analyse
long timeSpent = System.currentTimeMillis()-start

You'll have to put your code in a loop, to make it relevant. And you will probably notice that Java will increase the performance after some loops ;).

Upvotes: 3

UmNyobe
UmNyobe

Reputation: 22890

I will go for the option 1, and just reverse switch 

isDisabled(item) && !showDisabled

with 

!showDisabled && isDisabled(item)

If isDisabled(...) is as slow as you say it is better to test the faster case first. Now compared to the other option this is the most explicit and readable:

  • We do something is done for all items
  • we skip for items which validate some test
  • we do something for all other items.

It will be difficult to do more explicit. And the 3rd option is plain ugly.

Upvotes: 1

Hauke Ingmar Schmidt
Hauke Ingmar Schmidt

Reputation: 11607

Should I use cond1 && !cond2 or !(!cond1 || cond2)? Or should I check for the cond2 (show disabled items) before the loop?

Whatever expresses your thoughts best, i.e. whatever is more readable.

My main concern was speed.

Writing speed? Refactoring speed? Compilation speed? Development speed? Reading and understanding speed? Debugging speed? Execution speed?

You can't have all at once. In no language.

Upvotes: 1

Brian Agnew
Brian Agnew

Reputation: 272317

As always with these types of questions, you should measure this to determine if this itself is your bottleneck. If it is, then I would measure the alternatives. I suspect for the above scenario that it will make next to no difference for your alternatives, especially since you'll likely be doing something much more heavyweight with the list entries (displaying them? writing them to a db or file?)

The simplest way to measure this is to generate a sizable list, record the time (say, via System.currentTimeMillis()) before you process, process, and then record the ms (or seconds) taken.

Upvotes: 1

Related Questions