5
\$\begingroup\$

I made the following game in Python applying object oriented programming and I would like to know if there is something to improve. PS: My native language is Spanish, if you see something wrong written in the code let me know.

 The 23 matchsticks game is played between 2 participants.
 It consists of placing 23 matchsticks on the table and each player can remove
 1, 2 or 3 matchsticks per turn.
 The player who removes the last matchstick loses and is eliminated.
 Make a program that allows two participants to play the 23 matchsticks game.

 Example of execution:
 Start the game
 ||||| ||||| ||||| ||||| |||
 Player 1 remove: 3
 ||||| ||||| ||||| |||||
 Player 2 remove: 3
 ||||| ||||| ||||| ||
 Player 1 remove: 3
 ||||| ||||| ||||
 Player 2 remove: 1
 ||||| ||||| |||
 Player 1 remove: 3
 ||||| |||||
 Player 2 remove: 3
 ||||| ||
 Player 1 remove: 2
 |||||
 Player 2 remove: 1
 ||||
 Player 1 remove: 3
 |
 Player 2 is eliminated.
class MatchsticksGame:
    matchsticks = 23

    
    def __is_valid_input(self, player):
        return 1 <= player <= 3
            

    def __show_matchsticks(self):
        return '||||| '*(self.matchsticks//5) + '|'*(self.matchsticks%5)


    def __game_finish(self):
        return self.matchsticks <= 0
        

    def play(self):
        player2 = False
        print('Game starts')
        print(self.__show_matchsticks())

        while True:
            try:
                player = int(input(f'The player {player2+1} remove: '))

                if not self.__is_valid_input(player):
                    print('You can delete only between 1 and 3 matchsticks.')
                    continue

                self.matchsticks -= player
                print(self.__show_matchsticks())

                if self.__game_finish():
                    print(f'The player {player2+1} is eliminated.')
                    break

                player2 = not player2
            except:
                print('You can only enter numeric values.')


if __name__ == '__main__':
    game = MatchsticksGame()
    game.play()
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Types

Consider: player2 = False, player2 = not player2, and f'...{player2+1}...' Is player2 a boolean value that you can toggle between True and False or is it an numerical variable you can add 1 to? It shouldn't be both.

Using a player_number variable, initialized to 1, and then flipping it between 1 and 2 would be clearer. You can accomplish the flipping with a little bit of math: player_number = 3 - player_number. This is much less tricky than True+1 == 2!

Double underscore

Prefixing members with a double underscore has a special meaning. It is meant to help avoid name clashes between “private” attributes of base and derived classes. See the Reserved classes of identifiers for details. Since you are not using derived classes here, there is no need to use a double underscore prefix.

Variable naming

Reading return 1 <= player <= 3, it looks like you've a game that uses between 1 and 3 players. You are not validating the player; you've validating a number of match sticks to remove. Name the appropriately, such as number_to_remove.

Class Variables -vs- Instance Variables

What is matchsticks? Is it a class variable or an instance variable? Answer: they (plural) are both!

That is a somewhat confusing answer, but you've got a somewhat confusing situation here.

>>> game = MatchsticksGame()
>>> MatchsticksGame.matchsticks
23
>>> game.matchsticks
23

Both MatchsticksGame.matchsticks and game.matchsticks have the same value. Let's change one:

>>> MatchsticksGame.matchsticks -= 3
>>> MatchsticksGame.matchsticks
20
game.matchsticks
20

We changed one, and the other changed. They are the same variable!?

>>> game.matchsticks -= 3
>>> game.matchsticks
17
>>> MatchsticksGame.matchsticks
20

Nope!

What's going on? Initially, there is just the class variable MatchsticksGame.matchsticks with the value 23. When you reference game.matchsticks, it looks up the member in the game instance, can't find it, so looks up the member in the MatchsticksGame class. When you set/change the member value, with game.matchsticks -= 3 or self.matchsticks -= player, it fetches the value from the class, subtracts the indicated amount, and stores the value, creating the member of the instance object!

While this seems to work OK here, it fails in an unexpected way when you have a mutable object like a set or a dictionary.

Change this:

class MatchsticksGame:
    matchsticks = 23

    ...

to something like this:

class MatchsticksGame:
    INITIAL_MATCHSTICKS = 23

    def __init__(self):
        self.matchsticks = MatchsticksGame.INITIAL_MATCHSTICKS

    ...

Object Oriented Programming

You haven't really got object oriented programming here. You have an object with one data member (matchsticks) for state, and one method (play). True, you have 3 other helper methods, but they are pretty trivial -- you could easily substitute them into the play function without any loss of readability. In fact, the entire code could be converted almost unchanged into a single function without classes:

def play_matchsticks():

    def is_valid_input(num_to_remove):
        return 1 <= num_to_remove <= 3
            
    def show_matchsticks(matchsticks):
        return '||||| ' * (matchsticks // 5) + '|' * (matchsticks % 5)

    def game_finished(matchsticks):
        return matchsticks <= 0

    matchsticks = 23
    player = 2

    print('Game starts')
    print(show_matchsticks(matchsticks))

    while not game_finished(matchsticks):
        player = 3 - player
        while True:
            try:
                matchsticks_to_remove = int(input(f'The player {player} remove: '))
                if is_valid_input(matchsticks_to_remove):
                    break
                print('You can delete only between 1 and 3 matchsticks.')
            except:
                print('You can only enter numeric values.')

        matchsticks -= matchsticks_to_remove
        print(show_matchsticks(matchsticks))

    print(f'The player {player} is eliminated.')

if __name__ == '__main__':
    play_matchsticks()

The game itself actually has more state. It has a player which flips back and forth between two values, but the game instance doesn't store that; it is hidden as a local variable inside the play method.

If you wanted to really make this object oriented, you could have a game object initialized with two player objects. The game object would alternate asking each player object for and reporting the results. The player objects would themselves have the input() statement in a loop with try/except to ensure valid input is given. The game object would have a loop until the game was finished.

Something like the following:

class Player:
    def __init__(self, player_name):
        self.name = player_name

    def get_move(self) -> int:
        while True:
            try:
                matchsticks_to_remove = int(input(f'Player {self.name} removes matchsticks: '))
                if 1 <= matchsticks_to_remove <= 3:
                    return matches_to_remove
                ...
            except ValueError:
                ...

class MatchsticksGame:
    def __init__(self, player1, player2, initial_matchsticks=23):
        self.players = [player1, player2]
        self.turn = 0
        self.matchsticks = initial_matchsticks

    def _current_player(self) -> Player:
        return self.players[self.turn % 2]

    def _show_matchsticks(self) -> None:
        ...

    def _play_turn(self) -> None:
        player = self._current_player()
        matchsticks_to_remove = player.get_move()
        ...
        self.turn += 1

    def play(self) -> None:
        while self.matchsticks > 0:
            self._play_turn()
        ...

if __name__ == '__main__':
    p1 = Player("Alice")
    p2 = Player("Bob")
    game = MatchsticksGame(p1, p2)
    game.play()
\$\endgroup\$
10
  • \$\begingroup\$ You can use the built-in divmod() in show_matchsticks(). \$\endgroup\$ Feb 10 at 11:46
  • 1
    \$\begingroup\$ @Richard The divmod suggestion is a perfectly cromulent suggestion. You should post it as your own answer. \$\endgroup\$
    – AJNeufeld
    Feb 10 at 15:43
  • \$\begingroup\$ Nah, that small nitpick is not worth an answer. And your review covers most of the important points already. \$\endgroup\$ Feb 10 at 22:35
  • \$\begingroup\$ @AJNeufeld In the code you showed at the end the Game class would inherit from class Player or are they 2 independent classes? \$\endgroup\$ Feb 16 at 20:31
  • \$\begingroup\$ I have another question, something trivial, between method and method it is advisable to leave 1 or 2 enter. \$\endgroup\$ Feb 16 at 20:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.