HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

FizzBuzz with user input variables

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withuserfizzbuzzinputvariables

Problem

Yes, another FizzBuzz! I know you guys may be tired of these but I think it's a tradition stepping stone for beginners on this site.

Here are some notes on my thinking:

-
I tried to keep it as flexible as possible where the logic is deducted from the variables as much as possible. So one could potentially make a user input form on a website and let the user select all the values (numbers and words), and the code just does it.

-
I tried to avoid magic numbers as much as I know how. Not sure if 0 counts as a magic number.

-
I did not feel the need to add any comments as I feel it is self-explanatory. If you feel otherwise, please let me know.

Here is a link to PhpFiddle for your convenience.

\n";
}
?>

Solution

If your intention is to let users adjust the values used for the fizz and buzz values (3, and 5), then it is possible that they may choose numbers that break your logic quite badly. For example, if they chose '3' and '3', then we would expect all multiples of 3 to be 'FizzBuzz', but, instead, only multiples of 9 would be 'FizzBuzz'. This is because you need to find the lowest common multiple (LCM) of the fizz and buzz values, not just the product of them.

Either you need to calculate the LCM, or you can do them independently using a double test like:

if ($counter % $fizzNumber == 0 && $counter % $buzzNumber == 0) {
    echo $fizzWord . $buzzWord;
....


Notice how I have also removed the redundant (...) braces you had around the conditions. The % operator has a higher precedence than == so it does not need to be elevated.

Code Snippets

if ($counter % $fizzNumber == 0 && $counter % $buzzNumber == 0) {
    echo $fizzWord . $buzzWord;
....

Context

StackExchange Code Review Q#63722, answer score: 10

Revisions (0)

No revisions yet.