patterncMinor
FizzBuzz Challenge
Viewed 0 times
challengefizzbuzzstackoverflow
Problem
This bit of code is a small game to see how long you play a game of FizzBuzz without messing up.
Code:
The top of the code:
The
The
The
How it works:
My main concerns:
functions use Input and Output?
trouble?
Code:
The top of the code:
#include
#include
#include
typedef enum {false, true} boolean;
typedef enum {FIZZ, BUZZ, FIZZBUZZ} FB;
int n = 0; // the number that is being incremented
char input[256];
boolean user_turn(void);
FB n_fizz_buzz_fizzbuzz(void); // is n or fizz or buzz or fizzbuzzThe
main function:int main(void) {
boolean should_loop = true;
while(should_loop) {
n++;
printf("Number: %d\n", n);
should_loop = user_turn();
}
return 0;
}The
user_turn function:/*
Prompts the user for the answer
*/
boolean user_turn(void) {
FB correct = n_fizz_buzz_fizzbuzz();
scanf("%s", input);
if(!((strcmp(input, "fizzbuzz") == 0 && correct == FIZZBUZZ) || (strcmp(input, "fizz") == 0 && correct == FIZZ) || (strcmp(input, "buzz") == 0 && correct == BUZZ) || (atoi(input) == n && correct == n))) {
printf("Sorry! That is incorrect.\nScore: %d\n", n - 1);
return false;
}
return true;
}The
n_fizz_buzz_fizzbuzz function:/*
Returns the correct answer either:
FIZZBUZZ(2)
BUZZ(1)
FIZZ(0)
n
*/
FB n_fizz_buzz_fizzbuzz(void) {
if(n % 15 == 0)
return FIZZBUZZ;
if(n % 5 == 0)
return BUZZ;
if(n % 3 == 0)
return FIZZ;
return n;
}How it works:
- The number is logged to console.
- The user enters their answer, which is either FizzBuzz, Fizz, Buzz, or the number that was logged.
- If they are incorrect, the game ends.
- If they were correct, the number in incremented by 1 and this process goes back to step 1.
My main concerns:
- This might only apply to Java, but should the separate(non-main)
functions use Input and Output?
- Are the enumerated types appropriate, or are they causing too much
trouble?
- Is anything repetitive?
- Are there any inefficiencies?
- Are there any inconsistencies?
- Are there any bad practice
Solution
My suggestions:
-
Use
-
Split the long line in
-
Don't use global variables where locals and function parameters can do the job with ease.
-
If you need to use file-scope globals, declare them
-
I suggest to use a
I'm sorry to say, I respectfully disagree with the three points in your self-code-review, with regards to this particular program.
Example of Linux source code: github.com/torvalds/linux/blob/master/kernel/events/core.c Linus gives some exceptionally good advice about coding style, which is well worth reading for its humour and wisdom. Some of the best bits:
First off, I'd suggest printing out a copy of the GNU coding standards, and NOT read it. Burn them, it's a great symbolic gesture.
if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."
-
Again, I think the K&R style favors putting a short
-
In this case, I think the
-
Use
fgets instead of the unsafe scanf("%s",...)-
Split the long line in
user_turn() across several lines.-
Don't use global variables where locals and function parameters can do the job with ease.
-
If you need to use file-scope globals, declare them
static to avoid trouble when linking.-
I suggest to use a
for loop instead of the while loop:for (n=1; should_loop; n++) {I'm sorry to say, I respectfully disagree with the three points in your self-code-review, with regards to this particular program.
- The creators and inner circle of C (Kernighan, Ritchie, Thompson, Pike, et.al) don't use braces around single statements. While this indeed can be a trap for the novice, I suggest that an experienced programmer had better follow the K&R "standard" coding style. The Linux source code, for example, seems to favor single-statements without blocks. As for me, I made a C pre-processor which adds braces automatically, getting the best of both world IMO. Code sample: sam.nipl.net/eg/spectrum
Example of Linux source code: github.com/torvalds/linux/blob/master/kernel/events/core.c Linus gives some exceptionally good advice about coding style, which is well worth reading for its humour and wisdom. Some of the best bits:
First off, I'd suggest printing out a copy of the GNU coding standards, and NOT read it. Burn them, it's a great symbolic gesture.
if you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."
-
Again, I think the K&R style favors putting a short
enum in a single line, as you originally did. Well, I was going to reference the Plan 9 source code, but they seem to prefer doing it your way, splitting even the shortest enum over four lines!! I checked "The Practice of Programming" and in this case, Kernighan and Pike seem to agree with me, they do it in one line for a short enum and split things up for a longer enum with comments (which makes sense).-
In this case, I think the
n++ might be overlooked in the middle of a printf. I'd use a for loop, it's good to keep all the loop control stuff in one place.Code Snippets
for (n=1; should_loop; n++) {Context
StackExchange Code Review Q#79389, answer score: 4
Revisions (0)
No revisions yet.