Quindici

Overwriting Equals.

After yesterday’s success in testing the PlayerFactory I was ready to start moving on to complete the rest of my iteration.

However, early this morning I discovered that overwriting equals is actually a code smell and should be avoided whereever possible.

How then can we test the creation of two new instances of players. The answer is another code smell unfortunately, but in this case allowable. instanceof.

The tests for PlayerFactory now look like this, and the overwriting of equals and hashcode have been removed.

@Test
public void canCreateTwoHumanPlayers() {
    PlayerFactory pfactory = new PlayerFactory();
    assertTrue(pfactory.create(1).get(0) instanceof Human);
    assertTrue(pfactory.create(1).get(1) instanceof Human);
}

@Test
public void canCreateAHumanAndComputerPlayer() {
    PlayerFactory pfactory = new PlayerFactory();
    assertTrue(pfactory.create(2).get(0) instanceof Human);
    assertTrue(pfactory.create(2).get(1) instanceof DumbComputer);
}

Mateu also pointed out another code smell to me today, one that I hadn’t realised. I’m writing here in the hope that I’ll remember it!

public Game(List<Player> players) {
    this.players = players;
    this.currentPlayer = players.get(0);
    //   this.player1 = currentPlayer = players.get(0);
    //   this.player2 = players.get(1);
}

Assigining player1 to players.get(0) implied that there was a restriction and assumption of knowledge in the game. Mateu and hence this was a code smell, almost like repetition. The commented out code will be removed in favour of the more robust this.players = players. This allows for the possibility of more than two players in the game of tictactoe.