Thoughts and implementation on that daft FizzBuzz interview question

G'day:
I'm not quite sure where I'm gonna go with this article yet. Usually when I start an article like this it ends up being a bit rubbish, so... well sorry if it turns out this way this time too.

Right so there was a conversation on the CFML Slack channel last week stemming from someone deciding to use the old FizzBuzz question as part of a job interview. I'm not a great proponent of such things in interviews, but then back when I used to conduct such things I was shit at doing 'em anyhow, so what do I know?

But I don't think it's a sensible measure of anything to make someone code something in the actual interview: interviews can be high-stress environments, with restricted time-framing, and I don't think that reflects the general programming environment. Well not one I'd want to work in, anyhow. I've once been asked to design a database - in an interview for a developer role, I hasten to add - I just said "no, I'm not going to do that; I don't think that's a good use of anyone's time, and it casts you in a bad light for asking, and me in a bad light as I fumble around trying to do something that might normally take hours in only a few min. So: no" (that's a paraphrase, but it is the general gist of how the conversation went). I think perhaps had I actually wanted the job I might have humoured them. Maybe. It's dumb-arse anyhow.

Fortunately in this fizzbuzz situation the person conducting the interview was like-minded, and told the candidate to take it home with them and flick something back later. That's reasonable.

But what should one look for in a situation like this? And what should one deliver as a solution?

Firstly... in case you are like me and had no idea what people were on about when they started talking about FizzBuzz, this is the question:

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".

(from http://wiki.c2.com/?FizzBuzzTest)

The Slack channel got to messing around, and as one can probably imagine, shortly people starting answering the question. A typical answer might be like this:

for ($i=1; $i <=100; $i++) {
    $s = "";
    if ($i % 3 == 0) $s .= "fizz";
    if ($i % 5 == 0) $s .= "buzz";
    
    echo $s ?: $i;
}

That's simple and to the point, but I would not be impressed with that answer. Why? It's not reusable at all. OK, cool, the question didn't ask for re-use. That being the case, why bother with the code? Just use the output:

echo "12fizz4buzzfizz78fizzbuzz[etc]buzz";

If yer not needing a reusable answer... don't piss about writing code to do it. Just answer it. If I was in the position to discuss this with the interviewer - rather than just submit the answer - I would indeed consider starting with that answer there. Not to be a smart arse, but to let them know I think about that sort of thing.

But I'd then go on to point out some flaws in the question and in the answer. I think it's reasonable to provide slightly reusable code, not least of all so one can test it. One could test a file that simply has that statement in it, but it's not so orthodox to do so.

I put the answer in a function:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Then quickly realise that's still not very testable as testing frameworks usually expect code to be in classes. So I decide to do that: put it in a class.

However another thing I'd be wanting to do in this exercise is to demonstrate "test first" as a developmental approach. For this I just decided to use PHPSpec for a change today, as I heard someone mention it and thought "yeah, all right...". To get that strummed-up I need a composer.json file:

{
    "name" : "fizzBuzzChallenge",
    "require-dev" : {
        "phpspec/phpspec" : "^3.0"
    },
    "config": {
        "bin-dir": "bin"
    },
    "autoload": {
        "psr-4" : {
            "me\\adamcameron\\fizzBuzzChallenge\\" : "src/"
        }
    }
}

(some of that's from the PHPSpec docs, the rest is just "well I'll need to put the code somewhere, so might as well PSR-4 it and namespace it etc too).

Following the PHPSpec docs still, I start by describing my spec:

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec desc me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
Specification for me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\spec\FizzBuzzPuzzleSpec.php.


C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  11  - it is initializable
      class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle does not exist.

                                      100%                                       1
1 specs
1 example (1 broken)
25ms


  Do you want me to create `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle`
  for you?
                                                                         [Y/n]

Class me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle created in C:\src\php\php.local\src\puzzle\fizzbuzz\src\FizzBuzzPuzzle.php.

                                      100%                                       1
1 specs
1 example (1 passed)
27ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

So that's all quite cool. Notice that it created the spec class for me, and it also created the actual class for me too.

Now I have this:

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzPuzzleSpec extends ObjectBehavior
{
    function it_is_initializable()
    {
        $this->shouldHaveType(FizzBuzzPuzzle::class);
    }
}

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzPuzzle
{
}

Now I need to spec-out my function (test first remember!):

class FizzBuzzPuzzleSpec extends ObjectBehavior {

    function its_getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue() {
        $this->getFirst100FizzBuzzNumbers()->shouldBe("12fizz4buzzfizz78fizzbuzz[etc]buzz");
    }
}

(It's not part of my requirement to spec-out the return type of the class, so I got rid of that one).

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers not found.

                                      100%                                       1
1 specs
1 example (1 broken)
227ms


  Do you want me to create
  `me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers
  ()` for you?
                                                                         [Y/n]

  Method me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle::getFirst100FizzBuzzNumbers() has been created.

me\adamcameron\fizzBuzzChallenge\FizzBuzzPuzzle
  10  - its getFirst100FizzBuzzNumbersMethodReturnsTheExpectedValue
      expected "12fizz4buzzfizz78fizzbuzz...", but got null.

                                      100%                                       1
1 specs
1 example (1 failed)
29ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

Now I have the method skeleton as well:

public function getFirst100FizzBuzzNumbers()
{
    // TODO: write logic here
}

Which I replace with my implementation:

function getFirst100FizzBuzzNumbers(){
    return "12fizz4buzzfizz78fizzbuzz[etc]buzz";
}

Which works:

C:\src\php\php.local\src\puzzle\fizzbuzz>bin\phpspec run
                                      100%                                       1
1 specs
1 example (1 passed)
31ms


C:\src\php\php.local\src\puzzle\fizzbuzz>

However it's perhaps realistic to expect this thing to actually do the work dynamically instead of just returning a hard-coded string. So I shift my specification slightly to be "a function which takes an integer i and returns the first i FizzBuzz numbers.

The simplest spec for this is to just test that it returns a FizzBuzz number. The first FizzBuzz number is 1:

function its_getFizzBuzzNumbersMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumbers(1)->shouldBe(1);
}

Again I run the tests, and again it creates my method skeleton for me:

public function getFizzBuzzNumbers($argument1)
{
    // TODO: write logic here
}

Which I replace with enough code to pass.

public function getFizzBuzzNumbers($i) {
    return $i;
}

Now I'm intending to write enough of the algorithm to get me to 3, which is the first variation of the return value. I'm also gonna change the specification to return an array not a string. This is a nod to not assuming the use of the result, and an array is a better data type for a sequence of things. Coupled with this, I want to separate out the collection (the array) from the contents of the collection (FizzBuzz numbers). So I decide to spec a different function for now: getFizzBuzzNumber.

I create a spec:

function its_getFizzBuzzNumberMethodReturnsAFizzBuzzNumber() {
    $this->getFizzBuzzNumber(1)->shouldMatch('/\d+|fizz/');
}

And create a naive implementation of that:

public function getFizzBuzzNumber($i) {
    return $i;
}

And test the spec:

  16  - its getFizzBuzzNumberMethodReturnsAFizzBuzzNumber
      no match([array:1]) matcher found for [integer:1].

Erm... damn. What? This took a while to work out. PHPSpec seems to have forgotten that PHP is a dynamic, loosely-typed language, and seems to type-check the value before testing the pattern. If I returned a string, this spec worked:

public function getFizzBuzzNumber($i) {
    return (string) $i;
}

1 specs
1 example (1 passed)

Now I almost lived with that. I almost lived with returning the numeric FizzBuzz numbers as strings to make my pattern work. But that seemed arse-about-face. The spec was getting it "wrong", not my code. I chatted on the BDDLondon Slack Channel about how to test for "this OR that" (if not able to use a regex pattern"), and the answer was "cannae". This also had me thinking "I actually really hate methods that return two different types of thing, as it seems a bit messy. Then it occurred to me... it's not returning two different types of thing. It was returning one type of thing: it was returning a FizzBuzzNumber object. Aha! I was not after a collection of a mix of integers and strings "fizz", "buzz" or "fizzbuzz". They were all just FizzBuzzNumber objects. Cool.

So I spec-ed out a second class: FizzBuzzNumber. I'll spare you the stepped spec->code->repeat, and here's the end result. Spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzNumberSpec extends ObjectBehavior
{
    function it_isInitializable() {
        $this->beConstructedWith(1);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizz() {
        $this->beConstructedWith("Fizz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithBuzz() {
        $this->beConstructedWith("Buzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithFizzBuzz() {
        $this->beConstructedWith("FizzBuzz");
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isInitializableWithOtherInteger() {
        $this->beConstructedWith(2);
        $this->shouldHaveType(FizzBuzzNumber::class);
    }

    function it_isNotInitializableWithMultipleOfThree() {
        $this->beConstructedWith(6);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_isNotInitializableWithMultipleOfFive() {
        $this->beConstructedWith(10);
        $this->shouldThrow('\InvalidArgumentException')->duringInstantiation();
    }

    function it_returnsFizzWhenCreatingFromIntegerWithMultipleOfThree() {
        $this->beConstructedWith(1);
        $this::fromInteger(12)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(12)->__toString()->shouldBe("fizz");
    }

    function it_returnsBuzzWhenCreatingFromIntegerWithMultipleOfFive() {
        $this->beConstructedWith(1);
        $this::fromInteger(20)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(20)->__toString()->shouldBe("buzz");
    }

    function it_returnsFizzBuzzWhenCreatingFromIntegerWithMultipleOfFifteen() {
        $this->beConstructedWith(1);
        $this::fromInteger(30)->shouldHaveType(FizzBuzzNumber::class);
        $this::fromInteger(30)->__toString()->shouldBe("fizzbuzz");
    }

    function it_returnsAStringFromToString() {
        $this->beConstructedWith(1);
        $this->__toString()->shouldBeString();
    }
}

(I'm writing this part of this article about a week later, and I still can't stand seeing those underscores in my method names!)

But I like what PHPSpec has made me do here: define my FizzBuzzNumber's behaviour:

• it can be constructed with a value of 1 (a fairly baseline "it works");
• or a value of "fizz"
• or "buzz";
• or "fizzbuzz";
• or some other integer not matching those rules;
• but not an integer multiple of three (because there's no such FizzBuzzNumber... that'd be "Fizz");
• Likewise with a multiple of five
• (multiple of 15 is implicit from those two).
• However its fromInteger method can create an object from multiples of 3
• or 5.
• And if required to do so, returns a string value of itself (like for output).

That's a reasonable description (ie: specification) of a FizzBuzzNumber, I reckon?

The implementation is pretty simple:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzNumber {

    private $value;

    function __construct($value) {
        if (!preg_match('/\d+|fizz|buzz|fizzbuzz/', $value)) {
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }
        if (is_integer($value) && !($value % 3 && $value % 5)){
            throw new \InvalidArgumentException("$value is not a valid FizzBuzzNumber");
        }

        $this->value = $value;
    }

    static function fromInteger($i) : FizzBuzzNumber {
        $result = "";

        if ($i % 3 == 0) {
            $result .= "fizz";
        }
        if ($i % 5 == 0) {
            $result .= "buzz";
        }
        if (empty($result)) {
            $result = $i;
        }

        return new self($result);
    }

    function __toString() {
        return (string) $this->value;
    }
}

Notes:

• The constructor does nothing clever. It takes a valid FizzBuzzNumber value and rejects values that aren't valid. EG: 15 is not a valid FizzBuzzNumber: there's no such thing. The integer 15 might exist, but the FizzBuzzNumber equivalent is "fizzbuzz".
• The fromInteger method now holds all the logic of converting an integer to a FizzBuzzNumber, which is pretty much what we've been asked for: take the integers 1-100 and return their FizzBuzzNumbers.
• We want to treat FizzBuzzNumbers just like we would an integer etc. So if we output it, we just spit out its string value.

Right, so that's our numbers sorted out. Now we need a collection of them. This is pretty devoid of requirement now given the "fizz buzz" logic is within FizzBuzzNumber, so I made an arbitrary decision to take a slightly more complex route than perhaps I could have. I've decided to use a generator rather than just return the series at once. Why? This is for a job interview right, so it's perhaps good to demonstrate one's understanding of the language extends further than the basics. Also - as a discussion point - it's perhaps worthwhile to observe that this "fizz buzz" exercise is nothing more than a very naive Sieve of Eratosthenes, and that kind of lends itself to be handled via an "on demand" collection, a generator makes sense. But it's mostly one of those "because I can" sort of things. Plus it's dead easy in PHP.

Again, spec first:

<?php

namespace spec\me\adamcameron\fizzBuzzChallenge;

use me\adamcameron\fizzBuzzChallenge\FizzBuzzNumber;
use me\adamcameron\fizzBuzzChallenge\FizzBuzzSequence;
use PhpSpec\ObjectBehavior;
use Prophecy\Argument;

class FizzBuzzSequenceSpec extends ObjectBehavior {

    function its_class_IsInitializable() {
        $this->shouldHaveType(FizzBuzzSequence::class);
    }

    function it_returnsaGeneratorfromGetFizzBuzzSequence() {
        $this->getFizzBuzzSequence()->shouldHaveType(\Generator::class);
    }

    function it_fulfilsTheRequirement(){
        $lengthToTest = 100;
        $this->beConstructedWith($lengthToTest);

        $expected = array_map(function($i){
            return FizzBuzzNumber::fromInteger($i);
        },range(1, $lengthToTest));

        $this->getFizzBuzzSequence()->shouldIterateAs($expected);
    }
}

Notes:

• This tests that is is a generator.
• Plus here is the test for the actual requirement! Yup. I don't output it on the screen and ask the examiner to eyeball it manually: I provide a test for them so they can see that the computer has verified my work.

And the actual code is thus:

<?php

namespace me\adamcameron\fizzBuzzChallenge;

class FizzBuzzSequence {

    private $length;

    public function __construct($length=null) {
        $this->length = $length;
    }

    public function getFizzBuzzSequence() {
        $i = 0;
        while (is_null($this->length) || $i < $this->length){
            $i++;
            yield FizzBuzzNumber::fromInteger(($i));
        }
    }

    public function __invoke() {
        return $this->getFizzBuzzSequence();
    }
}

Simple, huh? OK, granted, not as simple as just a foreach loop, but it's not that much more complicated. The only twist is that I've just added an __invoke() method so that the object can be called as a method. Syntactical sugar.

OK, so if yer like me having that test there that proves the thing does the business ain't good enough. I also wrote a "manual test" to demonstrate it:

$seq = new FizzBuzzSequence(100);

foreach ($seq() as $fizzBuzzNumber) {
    echo $fizzBuzzNumber . PHP_EOL;
}

So we still get our simple foreach anyhow. But the implementation is hidden away in suitable classes.

And yeah it works:

C:\src\php\php.local\src\puzzle\fizzbuzz\public>php manualTest.php
1
2
fizz
4
buzz
fizz
7
8
fizz
buzz
11
fizz
13
14
fizzbuzz
16
17
fizz
19
buzz
fizz
22
23
fizz
buzz
26
fizz
28
29
fizzbuzz
31
32
fizz
34
buzz
fizz
37
38
fizz
buzz
41
fizz
43
44
fizzbuzz
46
47
fizz
49
buzz
fizz
52
53
fizz
buzz
56
fizz
58
59
fizzbuzz
61
62
fizz
64
buzz
fizz
67
68
fizz
buzz
71
fizz
73
74
fizzbuzz
76
77
fizz
79
buzz
fizz
82
83
fizz
buzz
86
fizz
88
89
fizzbuzz
91
92
fizz
94
buzz
fizz
97
98
fizz
buzz

C:\src\php\php.local\src\puzzle\fizzbuzz\public>

This is perhaps an overly-belaboured design of something simple, but I still think the design is sound, simple, and well-tested. I think it's an easy amount of work, and demonstrates a few sensible processes that are just as important to a developer job than actual code:

• be able to read the requirement, understand it, and deliver what's asked for (the hard-coded answer does this). This demonstrates one is paying attention and notices things.
• But question the requirement in a sensible way: is it just a one use thing? Is "100" cast in stone, or is it a parameter? This demonstrates one can then offer insight into where the client might have overlooked something, or "under-stated" their requirements due to using casual "human speak" when saying things like "I need the first 100 FizzBuzzNumbers". Questioning is always part of one's job.
• Any work one does should be spec'ed out (but only in a "just in time" fashion, not some horrendous waterfall). Know what yer doing before you set out to do it. It also helps collaboration with other devs, as it summarised things for them.
• and tested. All our work needs to be tested.
• Once one has teased-out the requirements from the initial statement, then one can crack on with the implementation, and piss around being "smart". Provided one can explain one's self.

That's the end of "my point" with this article.

---

As a digression though... obviously with these exercises "seasoned" devs might be wont to show-off how clever they are by making the solution as terse as they can. As if that's a good thing. It's not. But the conversation erred that way on the CFML Slack channel, and I rose to the bait.

Here's my foolishly terse solution to this, in a challenge I set for myself to implement the solution as a single expression in as fewer chars as possible:

[].set(1,100,"").reduce((s,_,i)=>s&(!i%15?"fizzbuzz":!i%3?"fizz":!i%5?"buzz":i),"")

(that only works on Lucee 5, btw. It will not work on ColdFusion)

That's absolutely ludicrous, and if anyone actually submitted that as an answer to the question they should be shot (yes, fine, a harsh penalty for failing a job interview).

But I was quite pleased that CFML can express the solution in one 84-char expression. I thought about trying to implement the same thing in PHP, but it simply can't do this sort of thing. Not being very OO and not doing a very good nod to FP doesn't help. CFML's abilities are def underrated here. In this example of "how to write shit code that'd get you shot".

Righto.

--
Adam