Last week: Ugly Trivia Game
To the Kata description
Scroll down to this weeks Kata description
As planned, I did the PHP version of this refactoring Kata over the week in multiple steps. First I needed test data for the “golden master”, the black box test
1. Step: Generate Test Data
The only input for the “Game runner” are random numbers (rand()), so I seeded the RNG with srand() to make them deterministic. Output is directly echo‘d, so I captured output with ob_start() and ob_get_clean().
Then I ran the program several times with different inputs (seed numbers), and created a data provider from the results.
2. Step: Output Stream
To get rid of the output buffering hack, I passed a stream resource to the game and replaced echo with fwrite($this->stream). The default stream would be STDOUT, but in tests I could replace it with fopen('php://memory', 'r+') and use stream_get_contents() for assertions.
3. Step: Rough Clean-up
There were some obvious code smells like public methods that should have been private, and methods that were never called. I refactored these without even trying to understand what the code was supposed to do. The golden master test assured that I would not change existing behavior.
3. Step: Extract Domain Objects
Now I started to figure out what actually happens. But instead of tangling my brain while mentally executing this mess of a program, I looked for candidates for domain objects that could be extracted. Then while moving more and more logic into these objects, the rules of the game got cleearer and clearer.
- question
- player:
- inPenaltyBox. interesting finding: players never get out of the box
- places
- coins
- category
- questions
4. Step: Refactor Findings
During refacoring I found lots of duplications that I could fix mostly with PhpStorm’s “extract method” feature. At one place I needed to fix a spelling mistake (“corrent” instead of “correct”) to do so, and fixed the test expectations in the golden master.
Also some methods had overcomplicated conditions or unlogical return types, now with a better understanding of the game, I could simplify a lot more than in the rough clean-up before.
5. Step: New Tests
Now I started to write targeted tests for the game logic by creating specific scenarios instead of using random input. For example:
- testCoinsForRightAnswer
- testPenaltyBoxForWrongAnswer
- testLeavePenaltyBoxWithOddRoll
- testStayInPenaltyBoxWithEvenRoll
- testGameIsFinishedWithSixCoins
During that phase, I also refactored the tests a lot to keep them clear and understandable.
6. Step
Now that I had covered the game logic with meaningful tests, I finally disabled the golden master and fixed the bugs in the game that I found before (like that players never leave the penalty box). Of course starting with a failing test for that condition!
7. Step
I was curious if it would be possible to completely decouple output from the game logic. I already had replaced the stream resource with an Output interface:
interface Output
{
public function writeln(string $line);
}
With implementations StreamOutput, ArrayOutput and NullOutput.
But I thought that something like this was still too specific and would usually not be mixed with the game logic:
$this->output->writeln("Question was incorrectly answered");
What I ended up with was decorators for the Category/Categories and Player/Players classes. All important methods for events in the game were in these classes, so I first moved all output there, then into the new decorators.
Example:
public function moveToPenaltyBox()
{
$this->output->writeln("Question was incorrectly answered");
$this->player->moveToPenaltyBox();
$this->output->writeln($this->name() . " was sent to the penalty box");
}
The game class now had two constructors.
A primary one:
public function __construct(Players $players, Categories $categories)
{
$this->players = $players;
$this->categories = $categories;
}
And a secondary one that creates a game with output and four categories, just as before:
public static function createWithOutput(Output $output)
{
$popQuestions = $scienceQuestions = $sportsQuestions = $rockQuestions = [];
for ($i = 0; $i < 50; $i++) {
$popQuestions[] = new Question("Pop Question " . $i);
$scienceQuestions[] = new Question("Science Question " . $i);
$sportsQuestions[] = new Question("Sports Question " . $i);
$rockQuestions[] = new Question("Rock Question " . $i);
}
return new Game(
new AnnouncingPlayers(new DefaultPlayers, $output),
new AnnouncingCategories(
new DefaultCategories(
new DefaultCategory("Pop", ...$popQuestions),
new DefaultCategory("Science", ...$scienceQuestions),
new DefaultCategory("Sports", ...$sportsQuestions),
new DefaultCategory("Rock", ...$rockQuestions)
),
$output
)
);
}
This extra mile was not really necessary but it was a good exercise and this way I ended up with a quite readable and not cluttered game class. Judge yourself: Do you understand the game logic by reading these methods?
public function isPlayable()
{
return count($this->players) >= 2;
}
public function add($playerName)
{
$this->players->add(
new DefaultPlayer($playerName)
);
}
public function roll($roll)
{
$this->currentPlayer()->roll($roll);
if ($this->currentPlayer()->isInPenaltyBox()) {
if ($roll % 2 === 0) {
$this->currentPlayer()->stayInPenaltyBox();
return;
} else {
$this->currentPlayer()->moveOutOfPenaltyBox();
}
}
$this->currentPlayer()->moveBy($roll);
$this->currentCategory()->nextQuestion();
}
public function rightAnswer()
{
if (!$this->currentPlayer()->isInPenaltyBox()){
$this->currentPlayer()->takeCoin();
}
$this->nextPlayer();
}
public function wrongAnswer()
{
$this->currentPlayer()->moveToPenaltyBox();
$this->nextPlayer();
}
public function isFinished()
{
foreach ($this->players as $player) {
if ($player->coins() === 6) {
return true;
}
}
return false;
}
private function currentCategory() : Category
{
return $this->categories->atIndex($this->currentPlayer()->location() % 4);
}
private function currentPlayer() : Player
{
return $this->players->atIndex($this->currentPlayer);
}
private function nextPlayer()
{
$this->currentPlayer = ($this->currentPlayer + 1) % count($this->players);
}
You might have noticed that I instantiate a DefaultPlayer in the add() method. That is the basic implementation without output decorator. This works because the Players collection object is an AnnouncingPlayers decorator and its add method also adds the AnnouncingPlayer decorator:
public function add(Player $player)
{
$this->getInnerIterator()->add(new AnnouncingPlayer($player, $this->output));
$this->output->writeln($player->name(). " was added");
$this->output->writeln("They are player number " . $this->count());
}
Conclusion
This was a huge Kata but an interesting one with many aspects. It was a good example of “golden master” tests for legacy code. I found it really interesting how I discovered the game logic while applying standard refactoring techniques and it was much easier than trying to understand the whole code upfront. Removing unnecessary coupling (here: output and game logic) is also a valuable thing to practice.
Thirteenth Kata: Boolean Expression Engine
The objective of this kata is to create a lexer / parser that will accept boolean logic statements as a string, evaluate them and return the boolean result of the expression.
The requirements for the parser are to accept the value tokens1(for true) and0(for false), deal with the logical tokens&,|and!(representing the logical operations AND, OR and NOT respectively) and to support logical grouping using the tokens(and).Examples:
Input:(1)
Output:trueInput:
0
Output:falseInput:
(0&1)|1
Output:trueStretch Goal:
Add support for variables in the logic statement, allowing you to pass the class variable names representing a boolean state that it will lookup as part of the expression evaluation. e.g.parse('foo&1', array('foo' => 0)) == false.Credit: this kata was invented by Peter O’Callaghan – thank you!
This is going to be an interesting challenge, I take it as an opportunity to refresh my compiler building knowledge.
