Circular increment: Which is "better"?

Question

When you have a circular buffer represented as an array, and you need the index to wraparound (i.e., when you reach the highest possible index and increment it), is it "better" to:

return (++i == buffer.length) ? 0: i;

Or

return ++i % buffer.length;

Has using the modulo operator any drawbacks? Is it less readable than the first solution?

EDIT:

Of course it should be ++i instead of i++, changed that.

EDIT 2:

One interesting note: I found the first line of code in ArrayBlockingQueue's implementation by Doug Lea.

Answer 1

Update: OP has admitted in a comment that it should have been pre-increment instead. Most of the other answers missed this. There lies proof that the increment in this scenario leads to horrible readability: there's a bug, and most people couldn't see it.

The most readable version is the following:

return (i == buffer.length-1) ? 0 : i+1;

Using ++ adds unnecessary side effect to the check (not to mention that I strongly feel that you should've used pre-increment instead)

What's the problem with the original code? Let's have a look, shall we?

return (i++ == N) ? 0 : i; // OP's original, slightly rewritten

So we know that:

• i is post-incremented, so when i == N-1 before the return statement, this will return N instead of wrapping to 0 immediately
  • Is this intended? Most of the time, the intent is to use N as an exclusive upper bound
• The variable name i suggests a local variable by naming convention, but is it really?
  • Need to double check if it's a field, due to side-effect

In comparison:

return (i == N-1) ? 0 : i+1; // proposed alternative

Here we know that:

• i is not modified, doesn't matter if it's local variable or field
• When i == N-1, the returned value is 0, which is more typical scenario

---

The % approach

Alternatively, you can also use the % version as follows:

return (i+1) % N;

What's the problem with %? Well, the problem is that even though most people think it's the modulo operator, it's NOT! It's the remainder operator (JLS 15.17.3). A lot of people often get this confused. Here's a classic example:

boolean isOdd(int n) {
   return (n % 2) == 1; // does this work???
}

That code is broken!!! It returns false for all negative values! The problem is that -1 % 2 == -1, although mathematically -1 = 1 (mod 2).

% can be tricky, and that's why I recommend the ternary operator version instead. The most important part, though, is to remove the side-effect of the increment.

See also

• Wikipedia: modulo operation

Answer 2

Don't ask me to choose between two options which both contain postincrement (*) mixed with expression evaluation. I'll say "none".

(*) Update: It was later fixed to preincrement.

Answer 3

Wouldn't the i++ % buffer.length version have the drawback that it keeps incrementing i, which could lead to it hitting some sort of max_int/max_long/max_whatever limit?

Also, I would split this into

i = (i++ == buffer.length) ? 0 : i;
return i;

since otherwise you'd most likely have a bug.