Learn to Code
John F. Dumas
contact me | resume | how it works | example programs | testimonials | main page
Feedback Examples: Two
one | two | three | four | five
► PROGRAMMING LANGUAGE:: C++
► ASSIGNMENT: The assignment was to author the program "buzz fizz", which outputs the numbers from 1 to 50 but follows these rules:
Here are the results your program should produce when executed:
1
2
buzz fizz
4
5
buzz
7
8
buzz
10
11
buzz
fizz
14
buzz
16
17
buzz
19
20
buzz
22
fizz
buzz
25
26
buzz
28
29
buzz fizz
fizz
fizz
buzz fizz
fizz
fizz
buzz fizz
fizz
fizz
buzz fizz
40
41
buzz
fizz
44
buzz
46
47
buzz
49
50
► FEEDBACK: Not a bad effort overall, but some of your output was not correct. Here are the results of executing your program along with notes where the output differs from what was expected:
1 2 buzz <-- should be "buzz fizz" 4 5 buzz 7 8 buzz 10 11 buzz fizz 14 buzz 16 17 buzz 19 20 buzz 22 fizz buzz 25 26 buzz 28 29 buzz <-- should be "buzz fizz" fizz fizz buzz <-- should be "buzz fizz" fizz fizz buzz <-- should be "buzz fizz" fizz fizz buzz <-- should be "buzz fizz" 40 41 buzz fizz 44 buzz 46 47 buzz 49 50
So, the numbers that gave it trouble were:
Notice what they have in common? They're all *both* divisible by three and also contain a three. That's why the output should be "buzz fizz" instead of just "buzz". Looking at your code:
#include <iostream>
int main()
{
int i = 1;
while(i <= 50)
{
if(i % 3 == 0)
{
std::cout << "buzz\n";
i++;
}
else if(i % 10 == 3 || i / 10 == 3)
{
std::cout << "fizz\n";
i++;
}
else if(i % 3 == 0 && (i % 10 == 3 || i / 10 == 3))
{
std::cout << "buzz fizz\n";
i++;
}
else
{
std::cout << i << "\n";
i++;
}
}
return(0);
}
See that first 'if' statement? When 'i' is 3 (for example) the program will hit:
if(i % 3 == 0)
{
...
And because the above is true (3 is divisible by 3), the code under the 'if' statement will be executed. So, the issue is that we never even get a chance to see if the number might also contain a 3, we just output "buzz" immediately.
Probably the easiest way to fix this is to re-order the statements so that the more specific check happens first (divisible by 3 and has a 3) as in:
#include <iostream>
int main()
{
int i = 1;
while(i <= 50)
{
if(i % 3 == 0 && (i % 10 == 3 || i / 10 == 3))
{
std::cout << "buzz fizz\n";
i++;
}
else if(i % 3 == 0)
{
std::cout << "buzz\n";
i++;
}
else if(i % 10 == 3 || i / 10 == 3)
{
std::cout << "fizz\n";
i++;
}
else
{
std::cout << i << "\n";
i++;
}
}
return(0);
}
Now the results are correct, however there are still a few possible improvements that can be made. Firstly, notice that this expression:
'i % 10 == 3 || i / 10 == 3'
Shows up twice in the if/else-if/else-if/else chain above. Also, this expression:
'i % 3 == 0'
Appears two times as well. While there is nothing wrong with this, it's easy to make a mistake when typing the same expression twice. One nice way to avoid this is to use boolean variables which simplifies things considerably:
#include <iostream>
int main()
{
int i = 1;
while(i <= 50)
{
bool divisibleByThree = (i % 3 == 0);
bool hasThree = (i % 10 == 3 || i / 10 == 3);
if(divisibleByThree && hasThree)
{
std::cout << "buzz fizz\n";
i++;
}
else if(divisibleByThree)
{
std::cout << "buzz\n";
i++;
}
else if(hasThree)
{
std::cout << "fizz\n";
i++;
}
else
{
std::cout << i << "\n";
i++;
}
}
return(0);
}
Lastly, notice that 'i++' happens in every one of the blocks. As a result, we can make things a bit more streamlined by moving 'i++' outside the if/else-if/else-if/else chain:
#include <iostream>
int main()
{
int i = 1;
while(i <= 50)
{
bool divisibleByThree = (i % 3 == 0);
bool hasThree = (i % 10 == 3 || i / 10 == 3);
if(divisibleByThree && hasThree)
std::cout << "buzz fizz\n";
else if(divisibleByThree)
std::cout << "buzz\n";
else if(hasThree)
std::cout << "fizz\n";
else
std::cout << i << "\n";
i++;
}
return(0);
}
We could also have used a 'for' loop instead of a 'while' which makes it easier to be certain your loop variable gets incremented each time:
#include <iostream>
int main()
{
for(int i = 1; i <= 50; i ++)
{
bool divisibleByThree = (i % 3 == 0);
bool hasThree = (i % 10 == 3 || i / 10 == 3);
if(divisibleByThree && hasThree)
std::cout << "buzz fizz\n";
else if(divisibleByThree)
std::cout << "buzz\n";
else if(hasThree)
std::cout << "fizz\n";
else
std::cout << i << "\n";
}
return(0);
}