Player data clump

[raaaahman]

Player Data Clump

The issue

Game:l50-53 shows an example of Data Clump: these are the players related data, and each index relates to a single player. Such indices need to be consistent across the arrays, which can eventually lead to bugs.

This commit introduces a new feature: now players can leave a game while running.

As outlined in the tests from this commit, removing a player from the Game::players array will mismatch the remaining players with the recorded data.

While this particular bug could be fixed by setting back the correct data in the Game::remove() method, now any changes on these data, or inclusion of a new type of data (for example, players may be assigned a color), would then need to be performed both in the Game::add() and Game::remove() methods. This might not be obvious to a new programmer joining the project.

Introducing a data holding class

The solution would be to group all the data referring to a same player in a dedicated data holding instance, representing this player.

To perform this operation safely, a first step is to encapsulate fields for Game::players, Game::places, Game::purses and Game::inPenaltyBox.

With all the data access encapsulated, we can nowmove these fields to the Player class. The Game::players will now hold references to Player instances in order to access this data.

Fixing the remove method

This refactor could have been stopped there, but the following commits outlines two new bugs that may occur when implementing the Game::remove() method:

-If a player leaves, the current player may be changed

• If the current players leaves, the current player must change to the next player

We can benefits from our players being instances here to use a reference to the current player, instead of an integer referring to its position in an array.

Cleaning up

This could also bring an end to this refactoring, although, the encapsulation of these change of the current player offers us a good opportunity to enforce separation of concerns, by moving all the code related to selecting the current player in a dedicated collection class

This in turn gives us the opportunity to level down all of the written tests to the PlayersList class scope, where the player removal feature belongs.