4
\$\begingroup\$

I have nearly one year of experience with C++, and it's been a month that I have been diving into Python. I came across this python game challenge on the internet called First to Five.

The rules are the following: There are two players. Each player writes a number. It can be any integer 1 or greater. The players reveal their numbers. Whoever chose the lower number gets 1 point, unless the lower number is lower by only 1, then the player with the higher number gets 2 points. If they both chose the same number, neither player gets a point. This repeats, and the game ends when one player has 5 points.

I really like to use OOP, most of my practices use it, and I would love to know a little bit about your opinions and thoughts about this code. What/How can I improve? Is it good? Am I using conventions and good practices?

class Game:
    game_win_point = 5;

    def __init__(self):
        self.victory = False;
        self.winner = "";
        self.player1 = Player();
        self.player2 = Player();

    def gatherInformation(self):
        """ Gathers information about both players playing the game before the game starts. """
        self.player1.setName(input("What's the name of player 1? >> "));
        self.player2.setName(input("What's the name of player 2? >> "));

    def check_number_input(self):
        """ Checks if both players entered valid input. """
        self.check_input(self.player1);
        self.check_input(self.player2);

    def check_input(self, player):
        """ Checks if 'player' entered a valid input. """
        try:
            player.setCurrentNumber(int(input("{}'s number: >> ".format(player.name))));
            if player.current_number <= 0:
                raise ValueError;
        except ValueError:
            while not isinstance(player.current_number, int) or player.current_number <= 0:
                print("{}, you have to input an integer greater than one!".format(player.name));
                try:
                    player.setCurrentNumber(int(input("{}'s number: >> ".format(player.name))));
                except ValueError:
                    player.setCurrentNumber(-1);

    def update_game_score(self):
        """ Updates the game score every round. """
        if self.player1.current_number == self.player2.current_number:
            # The numbers are the same.
            print("The numbers are the same and nobody gets a point! The score is: {}: {} points VS {}: {} points".format(p1.name, p1.score, p2.name, p2.score));
        else:
            # The numbers are diferent.
            greatest_number = max(self.player1.current_number, self.player2.current_number);
            self.who_scores(greatest_number, self.player1, self.player2);
            self.who_scores(greatest_number, self.player2, self.player1);

    def who_scores(self, greatest_number, ps, pl):
        """
        'greatest_number' stands for the maximum number inputted by player 1 and 2.
        'ps' stands for 'possible scorer' and it is the player who is supposed to get a score.
        'pl' stands for 'possible loser' and it is the player who is NOT supposed to get a score.
        Decides who scores a point.
        """
        if (greatest_number == ps.current_number):

            if pl.current_number == (ps.current_number - 1):
                # Then, 'possible_scorer' scores, because the 'possible_loser.current_number' is smaller by just 1.
                ps.goal(point=2);
            else:
                # Then, 'possible_loser' scores, because the 'possible_loser.current_number' is the smallest.
                pl.goal();

    def check_winner(self):
        """ Checks if there's a game winner. """
        if self.player1.score >= self.game_win_point:
            self.setVictory(True);
            self.setWinner(self.player1);

        if self.player2.score >= self.game_win_point:
            self.setVictory(True);
            self.setWinner(self.player2);

    def setVictory(self, victory):
        """ Sets the game victory variable to True if one of the players reach 5
        or more points. Otherwise, it remains False."""
        self.victory = victory;

    def setWinner(self, winner):
        """ Sets the specified winner player by the 'winner' attribute. """
        self.winner = winner;

class Player:
    def __init__(self):
        self.score = 0;
        self.name = "no_name";
        self.current_number = -1;

    def setScore(self, score):
        """ Setter/Mutator to set a player's score. """
        self.score = score;

    def setName(self, name):
        """ Setter/Mutator to set a player's name. """
        self.name = name;

    def setCurrentNumber(self, current_number):
        """ Setter/Mutator to set a player's current inputted number. """
        self.current_number = current_number;

    def goal(self, point=1):
        """ Increases this player's score. """
        self.score += point;

def main():

    print("Welcome to the game! This game was made for two players.");
    game = Game();

    game.gatherInformation();

    while not game.victory:

        # Getting input. If it is not a correct input, let's loop while the user gives uses a proper one.
        game.check_number_input();
        print("{} typed {}, and {} typed {}".format(game.player1.name, game.player1.current_number, game.player2.name, game.player2.current_number));

        # Checking who earns a points, if there's someone.
        game.update_game_score();

        # Displaying the results of this round.
        print("Results: {}: {} points VS {}: {} points.".format(game.player1.name, game.player1.score, game.player2.name, game.player2.score));

        # Checking if there is a winner.
        game.check_winner();

    print("Congratulations, {}! You're the winner.".format(game.winner.name));

if __name__ == "__main__": main();
\$\endgroup\$
4
\$\begingroup\$

1. Review

  1. In Python, there's no need to use a semi-colon to end a statement.

  2. Similarly, there's no need to put parentheses around the condition of an if statement.

  3. The docstrings on the methods are good, but there should also be docstrings on the classes.

  4. There's a bug in Game.update_game_score. If both players enter the same number, there's a NameError because the code uses p1 and p2 instead of self.player1 and self.player2.

  5. The message "you have to input an integer greater than one" isn't right: you can also enter the number 1.

  6. The Game.check_input method uses the methods and attributes of a single Player object. This suggests that it should be a method on the Player class.

  7. The check_input method is misleadingly named, because it gets the input as well as checking it.

  8. Instead of a separate gatherInformation function, it would make sense to gather the player's name in Player.__init__.

  9. There's a lot of repeated code of the form:

    do_something(self.player1)
    do_something(self.player2)
    

    This repetition can be avoided by putting the two players into a list, and iterating over the list:

    for player in self.players:
        do_something(player)
    
  10. In Python, we don't generally bother with setters or mutators, we just update the appropriate attribute. So instead of:

    self.setVictory(True)
    

    we'd just write:

    self.victory = True
    

    and avoid the need for a setVictory method. The reason is that if we later need to do something clever whenever an attribute is set, we can do that using the property decorator. We don't have to plan it in advance by writing getters and setters.

    (But later on we'll see that we don't even need a victory attribute.)

  11. The logic in check_input is quite complex and repetitive. It could be simplified so that there is just one call to input, and just one try: ... except:. If you follow my suggestion of making this a method on the Player class then it becomes:

    while True:
        try:
            self.number = int(input(f"{self.name}'s number: >>"))
            if self.number > 1:
                return
        except ValueError:
            pass
        print(f"{self.name}, you have to input an integer at least one!")
    

    Note that there is no need for the test not isinstance(player.number, int) because if the call to int succeeded then self.number must be an int.

  12. The logic in update_game_score is quite complex. One way to simplify it would be like this:

    diff = self.player1.number - self.player2.number
    if diff < -1:
        self.player1.score += 1
    elif diff == -1:
        self.player2.score += 2
    elif diff == 0:
        print("The numbers are the same and nobody gets a point!")
    elif diff == 1:
        self.player1.score += 2
    else:
        assert diff > 1:
        self.player2.score += 1
    

    The reason for writing it like this is to make it easy for a reader to check that we've covered all five cases.

  13. The code is now short enough that it makes sense to inline all the methods into main and remove the Game class.

2. Revised code

GAME_WIN_POINTS = 5             # Number of points needed to win.

class Player:
    """A player in the game, with attributes:

    name: str -- The name of the player
    number: int -- Most recently input number
    score: int -- Current score

    """
    def __init__(self, n):
        self.name = input(f"What's the name of player {n}? >> ")
        self.score = 0

    def __str__(self):
        return f"{self.name}: {self.score} points"

    def get_input(self):
        "Get input from the player and update player.number."
        while True:
            try:
                self.number = int(input(f"{self.name}'s number: >> "))
                if self.number > 0:
                    return
            except ValueError:
                pass
            print(f"{self.name}, you must input an integer at least one!")

def main():
    "Play the game."
    print("Welcome to the game! This game was made for two players.")
    player1, player2 = players = [Player(i) for i in (1, 2)]

    while True:
        # Get input.
        for player in players:
            player.get_input()

        # Update score.
        diff = player1.number - player2.number
        if diff < -1:
            player1.score += 1
        elif diff == -1:
            player2.score += 2
        elif diff == 0:
            print("The numbers are the same and nobody gets a point!")
        elif diff == 1:
            player1.score += 2
        else:
            assert diff > 1
            player2.score += 1

        # Display scores.
        print(f"Results: {player1} VS {player2}.")

        # Check for a winner.
        for player in players:
            if player.score >= GAME_WIN_POINTS:
                print(f"Congratulations, {player.name}! You're the winner.")
                return

if __name__ == "__main__":
    main()
\$\endgroup\$

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.