OOP Matchsticks Game [Update]

Question

I made the changes that were suggested to improve the program in the previous version. I would like to know if it is decent now or is there something to tweak.

class MatchsticksGame:
 def __init__(self, initial_matchsticks=23):
 self.players = ('Player 1', 'Player 2')
 self.player = None
 self.turn = 0
 self.matchsticks = initial_matchsticks
 def _current_player(self):
 return self.players[self.turn % 2]
 def _show_matchsticks(self):
 x, y = divmod(self.matchsticks, 5)
 return '||||| '*x + '|'*y
 def _get_move(self, player):
 while True:
 try:
 matchsticks_to_remove = int(input(f'{player} removes matchsticks: '))
 if 1 <= matchsticks_to_remove <= 3:
 return matchsticks_to_remove
 print('You can delete only between 1 and 3 matchsticks.')
 except ValueError:
 print('The value entered is invalid. You can only enter numeric values.')
 def _play_turn(self):
 self.player = self._current_player()
 self.turn += 1
 def _game_finished(self):
 return self.matchsticks <= 0
 def play(self):
 print('Game starts.')
 print(self._show_matchsticks())
 while self.matchsticks > 0:
 self._play_turn()
 matchsticks_to_remove = self._get_move(self.player)
 self.matchsticks -= matchsticks_to_remove
 print(self._show_matchsticks())
 if self._game_finished():
 print(f'{self.player} is eliminated.')
 break
if __name__ == '__main__':
 game = MatchsticksGame()
 game.play()

• Schmuddi: It's usually recommended on StackExchange sites to wait at least 24 hours before accepting answers so that answering questions doesn't become a race for the quickest one to reply. In other words, waiting for an appropriate amount of time before accepting encourages longer, more thorough answers.

Answer 1

There are a few things that you may want to improve.

1. Currently, the active player is determined in _current_player() by the turn number: if it's even, it's the turn of Player 1, otherwise, it's the turn of Player 2. This works, but it's not very explicit, and it would fail for any single- or multiplayer version of your game where the number of players is not two. Given that you only need the value determined by _current_player() to be able to print the right player name, I'd recommend using a different data structure to handle the players: deque from the collections module.

   A deque is basically a list that is optimized for inserting new elements at the beginning of the list. It also has a rotate(n=1) method that shifts the elements in the list by n positions. You can use this after every turn so that the first element of the deque contains always the current player. In this way, you can dispose of the class variables turn and player, as well as the methods _current_player() and _play_turn().

2. Your program doesn't properly handle cases in which the current player enters a number that exceeds the number of remaining sticks. For example, if there is only one stick left, the current player can still input 3. They will lose after all, but _show_matchsticks() will produce an output that looks as if there were still sticks remaining. You can solve this by changing the input validation in _get_move() to take the current number of sticks into account.

3. Your main game loop is a while loop with a losing condition, but you also use the method _game_finished() to determine the losing condition, and to explicitly break from the while loop even though it would stop at that point anyway. You can either remove the check for _game_finished(), or you can change the conditional while loop to a loop that will need to be halted explicitly (i.e. while True:). The latter version is an idiom frequently found for game loops or event loops. Given the simplicity of your game, however, I think it's more explicit and reader-friendly if you make the while loop determine whether the game has been lost, thus getting rid of _game_finished().

4. There are two calls to _show_matchsticks(), one before and one inside of your game loop. By rearranging them, you can reduce that to just one.

5. For reasons of brevity, I'd recommend to subtract the result of _get_move() from matchsticks without storing it in the intermediate variable matchsticks_to_remove.

6. Currently, you haven't limited the number of characters per line. PEP8, which is the ultimate reference for all issues concerning Python coding style, recommends a line length of 79 characters. This affects the lines with print() statements, which can be reformatted to include line breaks at appropriate places. For more complex programs, you may want to consider to store the strings in constants that are defined in one place anyway, because this will make changing the game output easier e.g. if you want to offer localized versions of your game.

Here's a version that combines these suggestions:

from collections import deque
class MatchsticksGame:
 def __init__(self, initial_matchsticks=23):
 self.players = deque(['Player 1', 'Player 2'])
 self.matchsticks = initial_matchsticks
 
 def _show_matchsticks(self):
 x, y = divmod(self.matchsticks, 5)
 return '||||| '*x + '|'*y
 def _get_move(self, player):
 max_value = min(3, self.matchsticks)
 while True:
 try:
 matchsticks_to_remove = int(
 input(f'{player} removes matchsticks: '))
 if 1 <= matchsticks_to_remove <= max_value:
 return matchsticks_to_remove
 print('You can delete only between 1 and '
 f'{max_value} matchsticks.')
 except ValueError:
 print('The value entered is invalid. You can only enter '
 'numeric values.')
 def play(self):
 print('Game starts.')
 while self.matchsticks > 0:
 print(self._show_matchsticks())
 
 player = self.players[0]
 self.matchsticks -= self._get_move(player)
 self.players.rotate()
 print(f'{player} is eliminated.')
if __name__ == '__main__':
 game = MatchsticksGame()
 game.play()

However, now there is only little justification left to use an object-oriented structure – a procedural version of your game is even more readable than that. This version will feature your methods _get_move(), _show_matchsticks(), and play() as separate module-level functions, and the game state will be passed between the functions as arguments. play() will be expanded by the initialization statements, and relabeled as main() because it's now the main top-level function.

In my opinion, this is pretty close to the most concise and most explicit version of your game that you can get:

from collections import deque
def show_matchsticks(matchsticks):
 x, y = divmod(matchsticks, 5)
 return '||||| ' * x + '|' * y
def get_move(player, matchsticks):
 max_value = min(3, matchsticks)
 while True:
 try:
 value = int(input(f'{player} removes matchsticks: '))
 if 1 <= value <= max_value:
 return value
 print(f'You can delete only between 1 and {max_value} matchsticks.')
 except ValueError:
 print('The value entered is invalid. You can only enter numeric '
 'values.')
def main(matchsticks=23):
 players = deque(['Player 1', 'Player 2'])
 print('Game starts.')
 while matchsticks > 0:
 print(show_matchsticks(matchsticks))
 player = players[0]
 matchsticks -= get_move(player, matchsticks)
 players.rotate()
 print(f'{player} is eliminated.')
if __name__ == '__main__':
 main()

• Lucio Mazzini: Hi, thanks for the suggestions, on what occasions is it appropriate to choose procedural programming over object oriented programming?
• Schmuddi: @LucioMazzini: I don't think that there's a general answer to that question. Classes have advantages, but these advantages come with increased code complexity as a cost. Your game uses a single class with only few attributes and methods that is instantiated only once, and there aren't any classes derived from the base class. Thus, using a class in this case doesn't really add anything that a procedural version can't offer as well. Have a look at the video that Richard-Neumann recommended in their answer, it's a good discussion of these issues.
• Schmuddi: @LucioMazzini: Having said that, I usually use classes even for toy projects like this as well, because I've been a fan of the conceptual beauty of object-oriented programming ever since I learned about it in the early 1990s. As the other answer says: It's useful to practice OOP, but it's also good to consider the alternative coding paradigms so that you learn the advantages and disadvantages of either.

Answer 2

The code looks pretty good to me.

Although I understand that you explicitly want to use OOP, you may want to compare your solution to a functional or multi-paradigmatic approach as well. You might learn something new. Especially because your class only exposes two methods, one of which is __init__().

Bonus points for using divmod(). ;)

The methods _current_player() and _game_finished() could be @propertys since they are not parameterized, and their return values depend only on the object's state without side-effects and with low workload.

In principle, _show_matchsticks() could also be a @property, but its name suggests a side-effect by using the word show. I would rename it to _matchsticks_str(), since it does not "show" anything, but just returns a string dependent on the object's state.