0

I am wondering if it is good practice to increment a factorial within the increment statement of a for loop rather than the body of the for loop?

Below shows the factorial being incremented multiplicatively within the increment statement:

for(Count = 1; Count <= 10; Factorial *= Count, Count++);

An alternative is incrementing the factorial within the body of the for loop:

for(Count = 1; Count <= 10; Count++)
{
    Factorial *= Count;
}

As the third parameter in a for loop is used for increments, is it a good or bad practice/code style to increment in this way? I believe it doesn't have any trade-off for readability + neatens up the code by saving some space, and although it doesn't change the outcome of the program, are there any negatives to using for loops in this way?

Ry-
  • 218,210
  • 55
  • 464
  • 476
Charlie Webb
  • 61
  • 7
  • *“I believe it doesn't have any trade-off for readability”* It does, and I bet most wouldn’t consider it a neatening. – Ry- Oct 21 '17 at 20:10
  • 1
    Many people wouldn't expect to see the `;` after the `for`, and might miss it or assume it's a mistake. Personally I like to separate the control of the loop from the actions taken. – Steve Oct 21 '17 at 20:12
  • There is a third version, which I think is better than your first, but worse than your second: `for(Count = 1; Count <= 10; Factorial *= Count++);` – mch Oct 21 '17 at 20:20
  • @Steve the convention I've seen for an empty `for` loop is to put the semicolon by itself on the following line, indented. That makes it pretty clear the loop body is intended to be empty. – Tom Karzes Oct 21 '17 at 20:20
  • @TomKarzes I think an empty `{ }` makes it clearer but then you may as well just include the body in there... – Steve Oct 21 '17 at 20:25

2 Answers2

4

Do not move loop body into loop header.

The separation of loop components into loop body and loop header greatly improves readability.

Your readers will be able to tell loop's "payload" (i.e. Factorial *= Count) from loop's control (everything else) with a quick glance at your second code snippet. Your first code snippet would have your readers scratch their head for a few seconds before they realize that (1) there is a semicolon at the end of the for loop, and (2) the payload of the loop is in the header.

The fact that the compiler would probably generate the same code in both cases should not be a factor at all.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Solid and concise reason on control vs. payload, you've convinced me it's better to keep it in the body. That, and as @chris2top said, it's likely actions will be more than 1 line, so it's beneficial to have the body available to append more code. Thanks. – Charlie Webb Oct 21 '17 at 20:22
2

Due to my experience its more common to do it in the body of the for loop, because most actions are more than 1 line. And for such "easy" code it doesn't make a big difference in readability. The only important thing for you is to stay consistent.

inxoy
  • 3,484
  • 2
  • 13
  • 21