1
\$\begingroup\$

I just recently started learning how to program and how to use python and after watching a few courses I made a simple Rock paper scissors game where basically the script randomly chooses Rock, paper or scissors then you make your choice and depending on what the program picked and what you picked you either win, lose or tie. This goes on until either you lose 3 times or win 3 times after which you'll be prompted for a rematch.

I was wondering how good is the code, I'd like Feedback as I want to know what I could have done differently to make the code better, cleaner and so on since I'm sure the code can be improved. I'll be using this sort of as a learning experience so I can better my programming knowledge.

import random
import time
import os
os.system("")

global vic_count
vic_count = 0
global loss_count
loss_count = 0
start = True
global rematchh
rematchh = True
CRED = '\033[91m'
CYEL = '\033[33m'
CGRE = '\033[32m'
CBLU = '\033[34m'
CTIL = '\033[3m'
CEND = '\033[0m'


def game_countinue():
    global vic_count
    global loss_count
    global rematchh
    while True:
        rematch = input("REMATCH? Y/N: ").upper()
        if rematch == "Y":
            vic_count = 0
            loss_count = 0
            rematchh = False
            return rematchh
        elif rematch == "N":
            break
        

def vcount():
    global vic_count
    vic_count += 1
    ccount()    
        
def lcount():
    global loss_count
    loss_count += 1
    ccount()
    
def ccount():
    print(CBLU + f"Player: {vic_count} Computer: {loss_count}" + CEND)    
 

def bot():
    bot_choice = ['Scissors', 'Paper', 'Rock']    
    bot_final_choice = random.choice(bot_choice)
    return bot_final_choice

   
def player():
     while True:
        player_choice = input(CTIL + "(R)ock, (P)aper or (S)cissors: " + CEND).upper()
        if player_choice == "P" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "P" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "P" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Paper")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break
        if player_choice == "R" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "R" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "R" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Rock")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break
        if player_choice == "S" and botchoice == "Rock":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CRED + 'You lose!' + CEND)
            lcount()
            break
        elif player_choice == "S" and botchoice == "Paper":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CGRE + 'You win!' + CEND)
            vcount()
            break
        elif player_choice == "S" and botchoice == "Scissors":
            time.sleep(1)
            print("Players choice: Scissors")
            time.sleep(1)
            print("Computers choice: " + botchoice)
            time.sleep(1)
            print(CYEL + 'Tie!' + CEND)
            ccount()
            break

while start == True:
    yes_or_no = input(CRED + "WARNING!!! THIS GAME IS SO HARD THAT IT WILL PHYSICALLY CHANGE YOUR BEING, THIS IS ONLY MEANT FOR THE TRULY HARDENED GAMERS, ARE YOU SURE YOU WANT TO CONTINUE? Y/N: " + CEND).upper()
    if yes_or_no == "N":
        break
    elif yes_or_no == "Y":
        time.sleep(1)
        print(CYEL + "WIN A FRIENDLY GAME OF ROCK PAPER SCISSORS, THE FIRST ONE TO GET 3 POINTS WINS!" + CEND)
        time.sleep(3)
        start = False
        rematchh = False

while start == False and rematchh == False:
    botchoice = bot()
    print("Choosing..")   
    time.sleep(1)
    print("Done choosing..")
    time.sleep(1)
    player()
    time.sleep(1)
    if vic_count >= 3:
        print(CYEL + "CONGRATULATIONS!" + CEND)
        rematchh = True
        game_countinue()
        
    elif loss_count >= 3:
        print(CRED + "YOU SURE GOT RAZZLE DAZZLED!" + CEND)
        rematchh = True
        game_countinue()

I'm sure the code can be improved, especially with detecting what the player chose and the computer chose to determine who wins, as I ended up just using a ton of if and elif statements that just clutter everything.

\$\endgroup\$
0

2 Answers 2

0
\$\begingroup\$

This looks great! Going forward, I would be sure to give variables/functions proper names. I used to use names like ccount and aamount and ended up being very confusing to read when sharing with others

\$\endgroup\$
0
\$\begingroup\$

Let's address the elephant in the room: the if-else ladder. When you see code duplication, extract common logic into a parametrized function and then call it with different parameters to alter the behaviour. In this case it can be done like so:

victory_table = {ROCK: (SCISSORS,), PAPER: (ROCK,), SCISSORS: (PAPER,)}

def determine_match_result(p1_choice, p2_choice):
    if p2_choice in victory_table[p1_choice]:
        return VICTORY
    elif p1_choice == p2_choice:
        return TIE
    else
        return DEFEAT

def resolve_match(player_choice, bot_choice):
    result = determine_match_result(player_choice, bot_choice)
    time.sleep(1)
    print(f"Players choice: {player_choice}")
    time.sleep(1)
    print(f"Computers choice: {bot_choice}")
    time.sleep(1)
    termcolor.cprint(result, "red")
    ccount()

In this example:

  • the entirety of game logic is contained in a dedicated function
  • game rules are recorded as a dictionary where each choice corresponds to a tuple of choices it can beat in a game (this solution can be extanded to play rock-paper-scissors-lizard-Spock or any other variant by modifying only one line)
  • plain strings are replaced with CONSTANTS that you can specify at the top of your script: python (as any other language) doesn't check for correctness of strings (it doesn't really know what to check for), so it's very easy to break the prorgam by misseplling a wrod, but with constants it's not a problem anymore:
VICTORY = "You win!"
TIE = "Tie!"
...
  • Instead of adding CYEL + and + CEND to every line, write a function that does it for you, or even better, use an existing library

As for the other problems:

import os
os.system("")

This line does nothing.

global vic_count

As well as this one. global keyword is used inside functions to access names out of their scope.

On the topic of global names: try to avoid them. They create a global state that makes reasoning and debugging much harder: function behaviour starts to depend on something out of its scope. You can do the same by passing required data as function parameters.

Naming is very important. It's much easier for people (and yourself) to read code when things are named based on what they actually do. Long descriptive names are better than short ones like ccount(). If it's a function, make it a verb that explains what it changes / returns. There is no way to know what the function bot() does by looking at its name. start may mean anything, is_game_started is more fitting. Try to find and change other names that don't explain their purpose to the reader.

if vic_count >= 3:
What is 3? Don't use magic numbers, replace them with constants:

SCORE_TO_WIN = 3
...
if vic_count >= SCORE_TO_WIN:

It's not only for readability: if you need to change the number, you no longer need to scan the code to find all the occurances, you just change the constant.


Overall very good for one of the first programs, keep it up!

\$\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.