Thanks for posting your code and asking for feedback. You implemented the code and it works. You probably tested it. That's already a good step ahead. I see student's code which was not tested.
Now for the hard part: feedback time. I see the following areas of improvement:
- separation of concerns: get things sorted.
- object orientation: if you can identify nouns in a verbal description of your game (write it down!), maybe those are a good choice for classes. If you can identify verbs in that description, maybe those are a good choice for methods or functions.
- Python details
The overview
Imagine a stranger like me getting an overview of your code. I'm going to look at the most left-aligned code, because that's what will be at the greatest abstraction level. I'm hoping to find classes (class X:) and functions (def f():) and a main entry point (if __name__ == "__main__":).
Here's what's top-level in your code:
imports. That's fine. It gives me an idea about what the code will use (timing and randomness) and not use (OpenCV, NumPy, Networking, ...)#get user's name. Hmm, why isn't that a method get_users_name() then?while True: An endless loop right at the beginning. Will that end well?#get user's choicedef get_player_input():. Why is it named different to what the comment says?#generate computer's choice. Why isn't that a method either? And what happened to the indentation?#determine the winner. Dito.get_player_input()- No main entry point.
At this point, it's clear to me that I need to read all the code in order to understand it. There is no game class which would control the games flow, no player class, no rules class, nothing.
What could I find instead?
class RockPaperScissors:
...
class HumanPlayer:
...
class ComputerPlayer:
...
if __name__ == "__main__":
...
That would tell me:
- It's a game that I know.
- It has two kinds of players.
- There's a main method which runs the thing.
Setting up the human player
At the beginning, you want to set up a human player. That's what #get user's name tells me and what the while True: loop does. That code has aspects of two things: it controls the flow of the game (because the game can be over) and it acts like a builder for a human object.
Let me try to separate that loop into a player class and a game class:
import sys
class HumanPlayer:
def __init__(self):
self.username: str = ""
def is_invalid(self) -> bool:
return self.username.strip() == ""
def wants_exit(self) -> bool:
return self.username == "q"
class RockPaperScissors:
def __init__(self):
self.human = None
def play(self):
self.human = self.set_up_human()
def set_up_human(self) -> HumanPlayer:
human = HumanPlayer()
while human.is_invalid() and not human.wants_exit():
human.username = input("What is your name? (Type 'q' if You want to end the game): ")
if human.is_invalid():
print('You have to type your name!')
if human.wants_exit():
print("Game over.")
sys.exit()
print(f"Hello {human.username}. Welcome to the game!")
return human
if __name__ == "__main__":
game = RockPaperScissors()
game.play()
Note how the while True: has disappeared and it's immediately clear that this is not an endless loop?
Player input
def get_player_input() sounds like a method for getting the player input, but actually it does everything: Human input, computer input and checking the outcome. Let's move the first part into the HumanPlayer class. For the computer, let's create a ComputerPlayer class with exactly the same function signature for duck typing reasons. And let's create a method for evaluating the match.
For the human:
class HumanPlayer:
def choose_from(self, items) -> str:
user_choice = ""
while user_choice not in items:
user_choice = input('Choose your item (paper, rock, scissors): ').strip()
if user_choice not in items:
print('You have chosen wrong item. Try again!')
time.sleep(.5)
print(f"You have chosen {user_choice}. Good luck!" + " The game begins!")
return user_choice
Also no endless loop here.
For the computer:
class ComputerPlayer:
def __init__(self):
self.username = "Computer"
def choose_from(self, items) -> str:
computer_choice = random.randint(0, len(items))
computer_move = items[computer_choice]
time.sleep(1.5)
print('Computer has chosen ' + computer_move)
return computer_move
Do you recognize that computer_choice and computer_move are hard to distinguish? I'll fix that later.
For the gameplay:
class RockPaperScissors:
def beats(self, item_a, item_b):
higher = {"rock": "scissors", "paper": "rock", "scissors": "paper"}
return higher[item_a] == item_b
def evaluate(self):
time.sleep(1.5)
if self.human.choice == self.computer.choice:
print("It is a draw!")
return
if self.beats(self.human.choice, self.computer.choice):
print(self.human.choice + ' beats ' + self.computer.choice + '. ' + 'You won!')
return
print('Oooops! ' + self.human.choice + ' is beaten by ' + self.computer.choice + '. ' + 'You lost!')
Note that I made a short function that can tell, whether one choice beat the other. It uses a dictionary. Read it and find out how it works.
Fine-tuning
- You know f-Strings. Let's use them everywhere.
- The random class has a built-in
choice() method. No need to calculate a number
- See the names in the
Computer.choose_from() method. They don't match. Let's fix that.
Final result
Look at this method:
def play(self):
self.human = self.set_up_human()
self.computer = ComputerPlayer()
items = ['paper', 'rock', 'scissors']
self.human.choose_from(items)
self.computer.choose_from(items)
self.evaluate()
Does that make sense for a Rock, Paper, Scissors game?
import random
import sys
import time
class HumanPlayer:
def __init__(self):
self.username: str = ""
self.choice = ""
def is_invalid(self) -> bool:
return self.username.strip() == ""
def wants_exit(self) -> bool:
return self.username == "q"
def choose_from(self, items):
user_choice = ""
while user_choice not in items:
user_choice = input("Choose your item (paper, rock, scissors): ").strip()
if user_choice not in items:
print("You have chosen wrong item. Try again!")
time.sleep(.5)
print(f"You have chosen {user_choice}. Good luck! The game begins!")
self.choice = user_choice
class ComputerPlayer:
def __init__(self):
self.username = "Computer"
self.choice = ""
def choose_from(self, items):
self.choice = random.choice(items)
time.sleep(1.5)
print(f"Computer has chosen {self.choice}")
class RockPaperScissors:
def __init__(self):
self.human = None
self.computer = None
def play(self):
self.human = self.set_up_human()
self.computer = ComputerPlayer()
items = ['paper', 'rock', 'scissors']
self.human.choose_from(items)
self.computer.choose_from(items)
self.evaluate()
def set_up_human(self) -> HumanPlayer:
human = HumanPlayer()
while human.is_invalid() and not human.wants_exit():
human.username = input("What is your name? (Type 'q' if You want to end the game): ")
if human.is_invalid():
print('You have to type your name!')
if human.wants_exit():
print("Game over.")
sys.exit()
time.sleep(.5)
print(f"Hello {human.username}. Welcome to the game!")
return human
def beats(self, item_a, item_b):
higher = {"rock": "scissors", "paper": "rock", "scissors": "paper"}
return higher[item_a] == item_b
def evaluate(self):
time.sleep(1.5)
if self.human.choice == self.computer.choice:
print("It is a draw!")
return
if self.beats(self.human.choice, self.computer.choice):
print(f"{self.human.choice} beats {self.computer.choice}. You won!")
return
print(f"Oooops! {self.human.choice} is beaten by {self.computer.choice}. You lost!")
if __name__ == "__main__":
game = RockPaperScissors()
game.play()
Is it perfect now?
Is that a perfect solution? Still not. There's room for improvement. E.g. I dislike:
- both, the human player and the computer player still have impact on the timing of the game, because they sleep themselves
- both players print messages to the console. That makes it hard to have a different UI for the game.
- the game class does not only know the rules of the game, it also has the UI built-in
- there's repetition for the items ("rock", "paper", "scissors") for which a typo could easily break the game.
However, the answer is already quite long and I'll leave it for now. Please come back with your next implementation and let us know how you improve from here.
