Do you consider this bad coding style?

I saw this code today :

 if(++counter == 10)
{

 //Do Something
 foo();

 }

I think this is bad style, but, is the execution compiler dependent aswell? say the counter is set to 8 before we get to this line, it's going to increment it, then compare 10 to 8, t开发者_开发知识库he value before, or compare 10 to 9, the value of counter after it got incremented?

What do you think SO? Is this common practice? bad style?

There's nothing compiler-dependent in the behavior of this code (besides possible overflow behavior). Whether it is a good style is a matter of personal preference. I generally avoid making modifications in conditionals, but sometimes it can be useful and even elegant.

This code is guaranteed to compare the new value to 10 (i.e. 9 is compared to 10 in your example). Formally, it is incorrect to say that the comparison takes place after counter gets incremented. There's no "before" or "after" here. The new value can get pre-calculated and compared to 10 even before it is physically placed into counter.

In other words, the evaluation of ++counter == 10 can proceed as

counter = counter + 1
result = (counter == 10)

or as

result = ((counter + 1) == 10)
counter = counter + 1

Note that in the first case counter is incremented before the comparison, while in the second case it is incremented after the comparison. Both scenarios are valid and perfectly possible in practice. Both scenarios produce the same result required by the language specification.

Operator precedence will always cause the increment to take place before the comparison. You may use parenthesis if you wish to make this very explicit, but I wouldn't call this bad coding style.

Personally I'd always separate this into two statements.

counter++;
if (counter == 10)
    DoSomething();

This way you don't need to think about what order things happen—there is no scope for confusion. It makes no difference to the generated code and when that is so, readability and maintainability concerns are always king.

It is well defined by the language standard, and whether it is a bad style or not is a matter of a personal preference, and of a context as well. I have one function using conditions similar to this, which I think looks and works very nice, and which I think would be less readable when the increment would be taken out of the condition.

const char *GetStat(int statId)
{
  int id = 0;
  if (statId==id++)
  {
    return "Buffers";
  }
  else if (statId==id++)
  {
    return "VBuffers";
  }
#ifndef _XBOX
  else if (statId==id++)
  {
    return "Reset factor";
  }
#endif
  else if (statId==id++)
  {
    return "CB Mem";
  }
  return "";
}

Note: the increments are actually not "performed" at all here, a decent compiler will eliminate the ++ done on id variable into constants.