which is the cleaner way to do this simple while?

Question

I'm learning C++ and I want to make clean and readable code. I was wondering which way is better? (this is supposed to make the factorial of 9)

First Method:

int main(){
    int i = 1,r = i;
    while (i < 10) {
       r *= ++i;
    }
}

Second Method:

int main(){
    int i = 1,r = i;
    while (i < 10) {
       i++;
       r *= i
    }
}

The first may be harder to understand but it's one less line. Is it worth it? What about performance? Obviously it wouldn't matter in such a trivial example but it would be a good practice to make fast code from the beginning.

Answer 1

That while can't get much simpler, but you can always switch to a for!

int r = 1;

for (int i = 1; i <= 10; i++) {
   r *= i;
}

Answer 2

Personally, of the two, I'd use the first one; it's readable and clear. If you were to use the second one, use the prefix increment (++i) on the standalone line anyway; it's slightly faster (ignoring potential compiler optimizations) in the general case.

Also, I'd probably use a for loop in this case -- I'm assuming there's a reason you're not.

There's not a good general rule for this -- if the line had more operations in it and function calls etc, it'd start being too complex. You get a good feel for complexity vs brevity as you get more experience.

And finally, I wouldn't worry about micro optimizations like this as you code, especially if you're just learning.

Answer 3

int factorial(int n) {
   int product = 1;
   for (int i = 1; i <= n; i++) {
      product *= i;
   }
   return product;
}

is way more readable (and generalized).

Answer 4

int main(){ 
   int i = 1,r = i; 
   while (i++ < 10) { 
      r *= i;
   } 

And forgetting some semi colons.

Answer 5

You could go a little more compact even...

int main() {
  int i = 10, r = 1;
  while (--i) r *= i;
}

not that that's really a good idea.

Answer 6

The second one, no doubt.

Mainly because if you're debugging the code, you always suspect the line r *= ++i; to be errorous.

Answer 7

int main()
{
    int answer = factorial(10);
}

int factorial(int n)
{
    if (n == 1) return 1;
    return n * factorial(n-1);
}

Answer 8

I think the other answers are better example of writing clear and readable code, but if you must have something that is very similar to the original code you posted, this might be a cleaner alternative.

int main(){
    int r, i = r = 1;
    while (i++ < 10) {
       r *= i;
    }
}

In my opinion, pre- and post-increment operators that are combined with assignments are one of the hardest code constructs to mentally parse. Pre- and post-increment in conditionals is a lot more common.

Answer 9

prefer ++i to i++

++i increments and gives back the new value i++ is doing 2 steps, creating a temp, incrementing the value and returning the temp old value

As such in general usage where you can use ++i the code will be simpler to understand. Both your examples can use ++i so there is little difference.

prefer for to while when you have simple loops similar to for each.

for (int i = 2, r=1; i <= 10; ++i) {
  r *= i
}

Answer 10

Why not fold the intended number in the while loop? Main thing for me is code readability because someone else is going to come back later and have to figure it out. Also, factorial's get really big fast, so you might want to use a long. Try to use the best type for the job.

long main() {
    return factorial( 10 );
}

long factorial( int n ) {
    long ans = (long) n;

    // error checking here
    if (n <= 0) { return -1; }
    while (n-- > 0) {
        ans *= n;
    }

Answer 11

As others has pointed out, try not to write 'smart code' but 'understandable code'. The solution given by msw is very good because the names of the variables shows what its purpose is.

Answer 12

Method #1, up to limitations of some debugging tools.

Compact code should be preferred, the problem is that you cannot debug aggregate statements on many of the popular cheap debugging tools. This often make you break long expressions, extract variables, etc.

In case of simple code as this one, compactness really outweights. You can see some of the motivation here: http://steve-yegge.blogspot.com/2008/02/portrait-of-n00b.html