CODEWITHSUNDEEP
javaoptimization
rootmeanclaire
rootmeanclaire

Reputation: 838

Is there a benefit to one of these statements

Are there any differences between these two snippets? Performance? Readability? Otherwise?

if (arg > 0) {
    for (int i = 0; i < arg; i++) {
        sb.append(">");
    }
} else if (arg < 0) {
    for (int i = 0; i < Math.abs(arg); i++) {
        sb.append("<");
    }
}

for (int i = 0; i < Math.abs(arg); i++) {
    if (arg > 0) {
        sb.append(">");
    } else if (arg < 0) {
        sb.append("<");
    }
}

Personally, I prefer the former, as it more clearly shows that arg is a constant.

Upvotes: 0

Views: 94

Answers (5)

maaartinus
maaartinus

Reputation: 46422

I'd say that both is pretty unreadable. Anyway, concerning speed:

  • First, measure if this part should be optimized at all. Most of the time, the answer is "no".
  • If the answer happens to be "yes": Try different variants and measure their performance.

The good thing is that the JIT loves simple and readable code.

I'd go simply for

for (int i = 0; i < Math.abs(arg); i++) sb.append(arg > 0 ? ">" : "<");

and hope

  • that the JIT recognizes that arg is a loop invariant
  • and Math.abs(arg) will be evaluated just once
  • and so will arg > 0 ? ">" : "<"

If I was crazy after optimization, I'd write

char c = arg > 0 ? '>' : '<'; // char instead of String
for (int i = Math.abs(arg); i > 0; i--) sb.append(x);

But actually there's a Guava solution:

sb.append(Strings.repeat(arg > 0 ? ">" : "<", Math.abs(arg));

Upvotes: 3

Ali
Ali

Reputation: 58461

I wouldn't use any of them; I would try one of the two snippets below.

First variant

    char c = arg > 0 ? '>' : '<';

    int  n = Math.abs(arg);

    StringBuffer sb = new StringBuffer(n);

    for (int i = 0; i < n; ++i) {

        sb.append(c);
    }

Here, my suggestions are:

  • Prefer primitives to Objects (char is likely to be faster than String).

  • Pre-allocate the buffer if you already know the size (by calling StringBuffer(int n)).

  • Prefer branch-free loops (better for readability, maintaince and speed).

Second variant

    char c = arg > 0 ? '>' : '<';

    int  n = Math.abs(arg);

    char[] content = new char[n];

    Arrays.fill(content, c);

    StringBuffer sb = new StringBuffer(n);

    sb.append(content);

Here, my suggestions are:

  • Prefer algorithms to self-written loops (the above snippet has no loops at all).

  • In particular, System.arraycopy() (called by sb.append(char[] )) is a native method which doesn't perform bound checks in each iteration.

I personally like this second version better because it is loop-free. It has the downside that it allocates and touches the memory corresponding to the content twice: First when the content is filled and then when it is copied over into to the StringBuffer.

Most important of all, profile your code to make sure you are not doing premature optimization.

"Want speed? Measure." (by Howard Hinnant)

Upvotes: 2

user207421
user207421

Reputation: 310909

I don't like either of them:

  • The first, because there is no need to call Math.abs() when you already know the sign.
  • The second, because it won't terminate if arg is negative. So it isn't equivalent to the first in any case. So you're comparing apples and oranges.

Upvotes: 1

Sam Hanley
Sam Hanley

Reputation: 4755

If as you stated arg is a constant, then the former would be more efficient, because you're only evaluating the if/else condition once, rather than evaluating it every time your loop iterates. However, if arg was not going to be a constant in every loop iteration, then you'd obviously want to go with the latter.

Upvotes: 2

Reed Oei
Reed Oei

Reputation: 1508

If arg is less than 0, the loop won't ever be run, since i, being equal to 0 at the start, is certainly not less than 0.

Therefore, the second option is slower if arg is less than 0, because it will run through all the options regardless of the fact that it won't ever be true.

Upvotes: 1

Related Questions