OOP-style FizzBuzz program in PHP

I've tried to write a configurable OOP-style FizzBuzz program in PHP for learning purposes. I hope everything is quite easy to understand.

• What do you think of the overall architecture?

• Are OOP principles kept well?

• What can be improved?

• How could error-handling and input-checking (i.e. currently you can break the program with ill-formed $multipliers array) be improved?

```
number = $number;
}

public function printMe()
{
return (string) $this->number;
}
}

class WordFizzBuzzItem implements FizzBuzzable
{
private $word;

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

public static function check($number, $multiplier)
{
return !($number % $multiplier);
}

public function printMe()
{
return $this->word;
}
}

class FizzBuzzPrinter
{
private $items = array();

public function add(FizzBuzzable $item)
{
$this->items[] = $item;
}

public function printMe()
{
$output = '';
foreach ($this->items as $item) {
$output .= $item->printMe();
}

return $output;
}

public function __toString()
{
return $this->printMe();
}

public function isEmpty()
{
return empty($this->items);
}
}

class FizzBuzzFactory
{
private static $multipliers = array();

public static function init(array $multipliers)
{
self::$multipliers = $multipliers;
}

public static function create($number)
{
$printer = new FizzBuzzPrinter();

foreach (self::$multipliers as $multiplier => $word) {
if (WordFizzBuzzItem::check($number, $multiplier)) {
$printer->add(new WordFizzBuzzItem($word));
}
}

if ($printer->isEmpty()) {
$printer->add(new NumberFizzBuzzItem($number));
}

return $printer;
}
}

class FizzBuzzIterator implements Iterator
{
private $f

First of: I like the iterator approach. We use the FizzBuzz problem as part of our interview process and to this date nobody went down that route.

The main points I have:

• Apparently your solution is quite over-engineered for a problem like this so I just assume you did it for the lack of a proper example (to be honest I usually find it quite difficult to come up with simple, comprehensive examples where the application of more complex designs doesn't seem over-engineered so fair enough :)).

• Your FizzBuzzFactory should not be static. This for example prevents running multiple FizzBuzz games simultaneously. Just create an instance with the specific multipliers as a member and store it as an additional member of your iterator. Static classes which hold states are evil (there are probably sensible applications for it but in general terms they are bad) as they create problems with unit testing, parallelization, re-use, modularity, etc.

• You have hidden the actual business logic in the WordFizzBuzzItem class as a static method. A more central place to have that function would be the FizzBuzzFactory as it is the one which effectively decides which FizzBuzzable to create.

-
The most common definition of the FizzBuzz game I know of is this:


Write a program that prints the integers 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"

However you have implemented this:


Write a program that prints the integers 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 the concatenation of both.

Effectively this means you only have to write array(3 => 'Fizz', 5 => 'Buzz') as rules instead of array(3 => 'Fizz', 5 => 'Buzz', 15 => 'FizzBuzz') so you saved yourself some typing work by reducing a case to a combination of the existing ones.

Why does it matter? If you consider the problem as the business logic given provided by a customer then approx. 5sec after the deployment of your solution the customer will come back and say: "Ah yes, I forgot, if it's divisible by 3 and 5 you have to print FixBugz because some of our legacy applications which we can't change have a typo in their parsing code." Now instead of just changing array(3 => 'Fizz', 5 => 'Buzz', 15 => 'FizzBuzz') into array(3 => 'Fizz', 5 => 'Buzz', 15 => 'FixBugz') you have to change a whole bunch of implementation code and unit tests.

In the end you have generalized a solution which might not fit the requirements because the requirements have changed (they always do).

StackExchange Code Review Q#33717, answer score: 12