My team recently worked on the popular Tennis Refactoring Kata.
The goal is mostly to implement a function that would return the score of a tennis game as a string based on the score of the 2 players. There are 3 situations to deal with:
I have mixed feeling about this exercise (did we really learn something?), but it was interesting to go through multiple implementations of the same problem
We jumped to fix the Tennis1 implementation, as 1) it was the first solution, 2) it seemed to be reasonably ok and 3) we spotted a few things that could be fixed:
Player class ?ifs, then range)won_point: this method does not check if playerName is actually a valid playerrange, when a simpler, shorter solution is possible # -*- coding: utf-8 -*-
from dataclasses import dataclass
@dataclass
class Player:
name: str
points: int = 0
def won_point(self):
self.points += 1
class TennisGame:
def __init__(self, player1Name, player2Name):
self.player1 = Player(player1Name)
self.player2 = Player(player2Name)
self.players = {self.player1.name: self.player1, self.player2.name: self.player2}
def won_point(self, playerName: str):
player: Player | None = self.players.get(playerName, None)
if player is None:
raise ValueError(f"{playerName} is not playing currently")
player.won_point()
def score(self) -> str:
result: str
# There are 3 cases to deal with:
# - the 2 players have an equal score
# - the 2 players have a different score, and both points are < 4
# - the 2 players have a different score, and one of the 2 players has a score >= 4
if self.player1.points == self.player2.points:
result = {
0 : "Love-All",
1 : "Fifteen-All",
2 : "Thirty-All",
}.get(self.player1.points, "Deuce")
# scores > 5 are allowed, and they would always return Deuce
elif max(self.player1.points, self.player2.points) < 4:
scores = {
0 : "Love",
1 : "Fifteen",
2 : "Thirty",
3 : "Forty",
}
result = f"{scores[self.player1.points]}-{scores[self.player2.points]}"
else:
score_difference = self.player1.points - self.player2.points
winner = self.player1.name if score_difference >= 1 else self.player2.name
is_advantage = abs(score_difference) == 1
result = f"Advantage {winner}" if is_advantage else f"Win for {winner}"
return result
To me, the best implementation is Tennis5.
Some other solution are clearly not good for team work and maintainability:
So to me, Tennis5 is the best starting implementation. All cases are dealt with, if there’s a bug it’s easy to fix.
It can slightly be improved, even though yes that’s quite a large function and some strings are repeated.
# -*- coding: utf-8 -*-
@dataclass
class Player:
name: str
points: int = 0
def won_point(self):
self.points += 1
class TennisGame:
def __init__(self, player1Name, player2Name):
self.player1 = Player(player1Name)
self.player2 = Player(player2Name)
self.players = {self.player1.name: self.player1, self.player2.name: self.player2}
def won_point(self, playerName: str):
player: Player | None = self.players.get(playerName, None)
if player is None:
raise ValueError(f"{playerName} is not playing currently")
player.won_point()
def score(self):
p1 = self.player1Score
p2 = self.player2Score
# When one of the scores is > 4, attempt to go back to the [0, 4] range
if max_score := max(p1, p2) > 4:
p1 -= (max_score - 4)
p2 -= (max_score - 4)
advantage_player1 = f"Advantage {self.player1.name}"
advantage_player2 = f"Advantage {self.player1.name}"
win_player1 = f"Win for {self.player1.name}"
win_player2 = f"Win for {self.player2.name}"
lookup = {
(0, 0): "Love-All",
(0, 1): "Love-Fifteen",
(0, 2): "Love-Thirty",
(0, 3): "Love-Forty",
(0, 4): win_player2,
(1, 0): "Fifteen-Love",
(1, 1): "Fifteen-All",
(1, 2): "Fifteen-Thirty",
(1, 3): "Fifteen-Forty",
(1, 4): win_player2,
(2, 0): "Thirty-Love",
(2, 1): "Thirty-Fifteen",
(2, 2): "Thirty-All",
(2, 3): "Thirty-Forty",
(2, 4): win_player2,
(3, 0): "Forty-Love",
(3, 1): "Forty-Fifteen",
(3, 2): "Forty-Thirty",
(3, 3): "Deuce",
(3, 4): advantage_player2,
(4, 0): win_player1,
(4, 1): win_player1,
(4, 2): win_player1,
(4, 3): advantage_player1,
(4, 4): "Deuce",
}
entry = (p1, p2)
if (entry in lookup):
return lookup[entry]
else:
raise ValueError("Invalid score.")