3

I was in a job interview and was asked to solve FizzBuzz with PHP.

Write a program that prints the numbers from 1 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”.

I never heard of FizzBuzz before but here is how I solved it because I didn't know modulo or how to use it.:

for ($i = 1; $i <= 100; $i++){
   if($i / 3 == round($i / 3) && $i / 5 == round($i / 5)){
      echo $i . " is FizzBuzz<br />";
   }
   else if($i / 3 == round($i / 3)){
      echo $i . " is Fizz<br />";
   }
   else if($i / 5 == round($i / 5)){
      echo $i . " is Buzz<br />";
   }
   else {
      echo $i."<br />";
   }
}

I googled and didn't find any solution with round and that got me thinking that maybe there is something wrong with it, this is one of the solutions I found that is close to mine:

for ($i = 1; $i <= 100; $i++){
   if($i % 3 == 0 && $i % 5 ==0){
      echo "FizzBuzz<br />";
   }
   else if($i % 3 == 0){
      echo "Fizz<br />";
   }
   else if($i % 5 == 0){
      echo "Buzz<br />";
   }
   else {
      echo $i."<br />";
   }
}

My code is working fine but is there anything wrong with it that I don't see?

Madian Malfi
  • 595
  • 1
  • 8
  • 26
  • 1
    There are often many different ways to solve a problem. If your solution works correclty, that should be appreciated. With that said, the solution you found would be more typical in that it uses the modulus operator which is commonly applied to situation where you need to know whether an integer can be evenly divided by another integer. – gview Aug 02 '17 at 07:32
  • Looking for duplicated or other solution : https://codereview.stackexchange.com/questions/tagged/fizzbuzz+php – Drag and Drop Aug 02 '17 at 07:36
  • @yoeunes makes a great point about your answer. You didn't read the instructions carefully, so your output is actually incorrect. – gview Aug 02 '17 at 07:37
  • @gview the interviewer didn't actually said that, I just quoted from anther blog to explain what is FizzBuzz incase someone doesn't know – Madian Malfi Aug 02 '17 at 07:41
  • Fair enough, but Fizzbuzz is a standard programming test question, and it includes that requirement, regardless of what was said. Take a look at @yoeunes answer. There are some interesting things he is doing that show he knows how to write professional caliber php code, which your code doesn't. To begin with there are established coding standards he uses that you do not. Read this: http://www.php-fig.org/psr/psr-2/ He also uses whitespace effectively, and provides more readable code – gview Aug 02 '17 at 07:47
  • His answer also shows some sophistication with the use of '===' rather than '==' – gview Aug 02 '17 at 07:50
  • @gview yes I can see, his code follows the standers and is more professional but still your first comment answers my question and his answer doesn't. – Madian Malfi Aug 02 '17 at 07:59
  • 1
    Just for fun, here's a couple of solutions that show off some fancy php tricks: https://3v4l.org/sfH03 and https://3v4l.org/4C1qb – gview Aug 02 '17 at 09:25

3 Answers3

9

Actually they are testing how you will solve such simple task. It should be increadibly optimized, the code shouldbe clean and easy readable.

Your version of code is not good. The version you've found in the internet is better, but it's not ideal from the point of the optimization.

Try to think how to get the goal with less actions.

Some tips:

  • do not use functions (such as range) for this task - it will only slow down the script execution time
  • use operator "for" for this task, do not use any other (while, do-while, foreach), because operator "for" the best fits in this case (you know how many iterations you need).
  • do not use round function, use modulus operator "%", because any function works slower than any operator
  • in the result you need to get code, in which the number of operations will be the least as possible (the number of "if" statements, the number of operators like "==" or "%"

    • Use 15 instead of % 3 && % 5 - less calculations - faster execution time.

My example of code:

for ($i = 1; $i <= 100; $i++) {
    if ($i % 15 == 0) {
        echo 'FizzBuzz<br>';
    } elseif ($i % 3 == 0) {
        echo 'Fizz<br>';
    } elseif ($i % 5 == 0) {
        echo 'Buzz<br>';
    } else {
        echo $i . '<br>';
    }
}
Sergej
  • 2,030
  • 1
  • 18
  • 28
  • 3
    In general I like your explanations, although I don't know that the micro optimization comments are appropriate or necessarily even accurate without profiling. – gview Aug 02 '17 at 08:11
7

Code style and lack of optimization gives impression of a newbie to the interviewer. My tips are:

  • Follow PSR-2
  • Never use else (restructure, use early returns/continues)
  • Always try to reduce the number of ifs (code complexity)
  • Use "identical" operators for comparison when dealing with integers and nulls (and any other type)
  • Do not use <br/> when HTML is never mentioned
  • Try to keep maintainability:
  • Extract match calculations in variables/functions, so they can be easily changed
  • Do not overcomplicate.
  • Try to optimize mathematically:
  • Use %15 instead of %3 and %5 check
  • You can also skip the above at all (check for %15), as you already have calculated that. Boolean operations are much faster.
  • Try not to calculate something twice.

IMHO, to follow all good practices your code should look like this:

for ($i = 1; $i <= 100; $i++) {
    $isFizz = (0 === $i % 3);
    $isBuzz = (0 === $i % 5);

    if (!$isFizz && !$isBuzz) {
        echo $i . PHP_EOL;

        continue;
    }

    if ($isFizz) {
        echo 'Fizz';
    }

    if ($isBuzz) {
        echo 'Buzz';
    }

    echo PHP_EOL;
}

Test

There is yet another tricky solution

for ($i = 1; $i <= 100; $i++) {
    switch ($i % 15) {
        case 3:
        case 6:
        case 9:
        case 12:
            echo 'Fizz';
            break;
        case 5:
        case 10:
            echo 'Buzz';
            break;
        case 0:
            echo 'FizzBuzz';
            break;
        default:
            echo $i;
            break;
    }

    echo PHP_EOL;
}
venimus
  • 5,907
  • 2
  • 28
  • 38
  • 'Never use else'? Can you please explain a bit more about this statement? is it due to a personal coding style preference or actually a way of performance optimisation? Thanks. – DAMIEN JIANG Dec 22 '21 at 02:51
  • 4
    It indicates a code smell, ie. violation of principles like "Tell,Don't Ask", "Single responsibility", code branching (n-path complexity). Many languages have "branch prediction" mechanism which will be executed (to mitigate the performance hit to some degree, which obviously indicates that it is better to avoid it). In addition it makes code less maintainable, testable and readable. They sometimes call it "poor man's polymorphism". Best practice is to only use "else" to assign default values and in the form of a ternary operator, although for simple vars best is to first assign, then override – venimus Dec 22 '21 at 18:46
  • It's rare I see actual clean code on PHP, but this is an example of it. Bravo! – Max Mar 01 '22 at 21:35
  • 1
    your tricky solution is great but it doesn't cover 12 which is a multiple of 3 and should print Fizz ;-) – SMASHED Apr 17 '23 at 22:00
  • yeah, you are right, i added it – venimus Apr 19 '23 at 15:24
4

if you read carefully it says "instead".

this is another short and clean solution of the FizzBuzz problem :

foreach (range(1, 100) as $number) {
    if(0 !== $number % 3 && 0 !== $number % 5) {
        echo $number.'<br>';
        continue;
    }

    if(0 === $number % 3) {
        echo 'Fizz';
    }

    if(0 === $number % 5) {
        echo 'Buzz';
    }

    echo '<br>';
}

the output is :

1
2
Fizz
4
Buzz
Fizz
7
8
Fizz
Buzz
11
Fizz
13
14
FizzBuzz
16
yoeunes
  • 2,927
  • 2
  • 15
  • 26
  • the interviewer didn't actually said that, I just quoted from anther blog to explain what is FizzBuzz incase someone doesn't know – Madian Malfi Aug 02 '17 at 07:45
  • 1
    I looked at your code and thought there was something wrong. I tested it out and it absolutely doesn't produce the output you claim to have. Your code displays the number every time it's not a multiple of 15 so the multiples of 5 and 3 do not produce Fizz or buzz. – Bevelopper May 31 '19 at 00:15
  • 1
    yes @Devoodoo catch, i forgot the condition, thank you – yoeunes May 31 '19 at 08:34