What's wrong with this FizzBuzz?

Question

The rules are unsurprisingly:

Write a program that prints the numbers from 0 to 100. But for multiples of three print “Fizz” instead of the number and for the multiples of five print “Buzz”. For numbers which are multiples of both three and five print “FizzBuzz”."

#include <stdio.h>

int main(void) {
    int i, j;
    i = j = 0;

    for (; i < 100 ; ++i, j = 0, putchar('\n')) {
        j += printf("%s", i % 3 ? "" : "Fizz");
        j += printf("%s", i % 5 ? "" : "Buzz");
        if (!j) printf("%d", i);
    }

    return 0;
}

If you were a job interviewer, would you be ok with an answer like this? Or would you ask for another solution that is not (ab)using the semantics of for and the return value of printf in order to do fewer operations?

Answer 1

The for loop seems fine to me; it isn't abusing any semantics. The semantics of the for loop are quite clear:

for (initialization; loop condition; increment)

Anyone who doesn't know this off the top of their head probably doesn't know the language as well as they think they do. Although you could have written this code using a while loop, it's just a matter of opinion whether that is better. In general, you can transform any for loop into a while loop. When I write a "complex" for loop, I tend to prefer to break each of these components into separate lines for readability (and also to keep the line from getting too long).

However, using the return value of printf the way you did is just wrong. It returns a negative value if the operation failed. You are adding arbitrary positive and negative return values together and checking to see whether the result is non-zero. That is incorrect, and thus decidedly unclever.

You could might say that the test condition could be modified to j > 0, but that's still not precisely correct because one of the printf calls might succeed and return, say, 5, while the other one might fail and return −5. Should the number be printed, or not?

So, while I have no problem with your choice to use a for loop here, I do not like what you've done in the body of that loop. I disagree with cHao's comment that this would be an instant no-hire. I'd probably assume you were a competent programmer and having fun with the inherently-boring FizzBuzz problem. However, I would expect you, with a bit of prompting, to see and begin to explain some of the problems with your chosen approach.

Aside from that…

• Keep in mind that code does not become more efficient just because you reduce the number of apparent instructions that you write in C. Optimizing compilers do not perform a literal translation of C instructions into assembly instructions. And even non-optimizing compilers may not be able to do so if they wanted to, because there isn't necessarily a 1-to-1 correspondence of instructions in both languages.

  In this case, you're not even gaining any efficiency with the way you've written the printf statements. In fact, the execution of printf is by far the slowest part of the program, so by any real-world performance metrics, avoiding calls to printf whenever possible would be the way to boost performance. Certainly it is not a win to replace a reasonably well-predicted conditional branch with an expensive and pointless function call!

  In fact, if you weren't trying to be overly clever, you could substitute puts for printf where you were outputting constant "Fizz" and "Buzz" strings, which would be even more efficient. (Some compilers like GCC will do this translation for you when they can, but they wouldn't be able to do it with the way you'd originally written the code, obviously.)

• Stylistically, I would prefer that you kept the variables to as narrow a scope as possible. Since at least C99, you have been allowed to declare variables within a for loop's scope, and this is what I would recommend that you do.

• Even more importantly, I prefer to see that variables are initialized at the point of their declaration whenever possible. It bothers me to see lines like:

  int i, j;
  i = j = 0;
  

  I don't like the attempt to combine initialization of multiple variables onto one line, either. Break them up into two lines. No excuses.