4
\$\begingroup\$

I began trying to learn python3 as my first programming language two days ago for a hobby and interest to teach my kids later on. I've been working on a program guessing a random number. I started with just guessing the number and built on to add two players then added a computer opponent to it as well. I would like for some constructive criticism on what i can do to improve what i have scratched together.

import random
import time

numToGuess = random.randint(1, 100)
p1NumOfGuess = 1
p2NumOfGuess = 1
c1NumOfGuess = 1
prevHigh = 100
prevLow = 1
p1 = int
p2 = int
c1 = int

print('1 or 2 Player?')
playerAmount = int(input())

if playerAmount == 1:
    print('Computer Player? [Y/N]')
    c1 = input()
    print('Player one enter your name')
    p1 = input()

else:
    print('Player one enter your name')
    p1 = input()
    print('Player two enter your name')
    p2 = input()
    print('Computer Player? [Y/N]')
    c1 = input()

print(p1, "Guess a Number between 1 and 100")
p1Guess = int(input())
if playerAmount == 1 and c1 != "y":
    while p1Guess != numToGuess:
        p1NumOfGuess += 1

        if p1Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p1Guess = int(input())

        elif p1Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p1Guess = int(input())

    print("Congrats", p1Guess, "is correct!")
    print(p1, 'took', p1NumOfGuess, 'guesses to guess correctly!')

elif playerAmount == 2 and c1 != "y":
    while p1Guess != numToGuess:
        p1NumOfGuess += 1

        if p1Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p1Guess = int(input())

        elif p1Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p1Guess = int(input())

    print("Congrats", p1Guess, "is correct!")

    numToGuess = random.randint(1, 100)
    print(p2, "Guess a Number between 1 and 100")
    p2Guess = int(input())

    while p2Guess != numToGuess:
        p2NumOfGuess += 1

        if p2Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p2Guess = int(input())

        elif p2Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p2Guess = int(input())

    print("Congrats", p2Guess, "is correct!")

    print(p1, 'took', p1NumOfGuess, 'guesses to guess correctly!')
    print(p2, 'took', p2NumOfGuess, 'guesses to guess correctly!')

    if p1NumOfGuess < p2NumOfGuess:
        print(p1, 'WINS!')

    elif p1NumOfGuess > p2NumOfGuess:
        print(p2, 'WINS!')

    else:
        print('Tied Game!')

elif c1 == "y" and playerAmount == 1:
    while p1Guess != numToGuess:
        p1NumOfGuess += 1

        if p1Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p1Guess = int(input())

        elif p1Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p1Guess = int(input())

    print("Congrats", p1Guess, "is correct!")

    numToGuess = random.randint(1, 100)
    c1Guess = random.randint(1, 100)
    print('Computer is guessing')

    while c1Guess != numToGuess:
        c1NumOfGuess += 1
        time.sleep(1.25)

        if c1Guess < numToGuess:
            print(c1Guess)
            print("Guess is to Low!""\n""Guess Again")
            prevLow = c1Guess
            c1Guess = random.randint(c1Guess, prevHigh)

        elif c1Guess > numToGuess:
            print(c1Guess)
            print("Guess is to High!""\n""Guess Again")
            prevHigh = c1Guess
            c1Guess = random.randint(prevLow, c1Guess)

    print("Congrats", c1Guess, "is correct!")

    print(p1, 'took', p1NumOfGuess, 'guesses to guess correctly!')
    print('Computer took', c1NumOfGuess, 'guesses to guess correctly!')

    if p1NumOfGuess < c1NumOfGuess:
        print(p1, 'WINS!')

    elif c1NumOfGuess < p1NumOfGuess:
        print('Computer WINS!')

    else:
        print('Tied Game!')

elif c1 == "y" and playerAmount == 2:
    while p1Guess != numToGuess:
        p1NumOfGuess += 1

        if p1Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p1Guess = int(input())

        elif p1Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p1Guess = int(input())

    print("Congrats", p1Guess, "is correct!")

    numToGuess = random.randint(1, 100)
    print(p2, "Guess a Number between 1 and 100")
    p2Guess = int(input())

    while p2Guess != numToGuess:
        p2NumOfGuess += 1

        if p2Guess < numToGuess:
            print("Guess is to Low!""\n""Guess Again")
            p2Guess = int(input())

        elif p2Guess > numToGuess:
            print("Guess is to High!""\n""Guess Again")
            p2Guess = int(input())

    print("Congrats", p2Guess, "is correct!")

    numToGuess = random.randint(1, 100)
    c1Guess = random.randint(1, 100)
    print('Computer is guessing')

    while c1Guess != numToGuess:
        c1NumOfGuess += 1
        time.sleep(1.25)

        if c1Guess < numToGuess:
            print(c1Guess)
            print("Guess is to Low!""\n""Guess Again")
            prevLow = c1Guess
            c1Guess = random.randint(c1Guess, prevHigh)

        elif c1Guess > numToGuess:
            print(c1Guess)
            print("Guess is to High!""\n""Guess Again")
            prevHigh = c1Guess
            c1Guess = random.randint(prevLow, c1Guess)

    print("Congrats", c1Guess, "is correct!")

    print(p1, 'took', p1NumOfGuess, 'guesses to guess correctly!')
    print(p2, 'took', p2NumOfGuess, 'guesses to guess correctly!')
    print('Computer took', c1NumOfGuess, 'guesses to guess correctly!')

    if p1NumOfGuess < p2NumOfGuess and p1NumOfGuess < c1NumOfGuess:
        print(p1, 'WINS!')

    elif p2NumOfGuess < p1NumOfGuess and p2NumOfGuess < c1NumOfGuess:
        print(p2, 'WINS!')

    elif c1NumOfGuess < p1NumOfGuess and c1NumOfGuess < p2NumOfGuess:
        print('Computer WINS!')

    else:
        print('Tied Game!')

else:
    print('Thanks for playing!')
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

You have a lot of duplicated (and triplicated) code. Learning how to write and use functions can help avoid this, but even without using functions your code can be significantly reduced.

But first, some initial(-ization) criticisms.

numToGuess = random.randint(1, 100)
p1NumOfGuess = 1
p2NumOfGuess = 1
c1NumOfGuess = 1
prevHigh = 100
prevLow = 1
  1. You are initializing variables way ahead of where they are used. At first, I thought prevHigh and preLow were unused variables. Initializing variables just as they are about to be used will help increase the cohesiveness of the code.

  2. You are initializing "guess" counter variables to 1. This is very unnatural. The counts should be initialized to 0, and incremented by 1 for each and every guess, including both the first and last guesses.

The next 3 initializations are more confusing:

p1 = int
p2 = int
c1 = int

I'm not certain what your intension was with these initializations. Perhaps you were trying to use Python3.6's type-hint declarations, such as p1 : int? But p1, p2 and c1 are used as str variables, not int.


print('1 or 2 Player?')
playerAmount = int(input())

if playerAmount == 1:
    print('Computer Player? [Y/N]')
    c1 = input()
    print('Player one enter your name')
    p1 = input()

else:
    print('Player one enter your name')
    p1 = input()
    print('Player two enter your name')
    p2 = input()
    print('Computer Player? [Y/N]')
    c1 = input()

Here is where we can start removing duplicate code. Regardless of the number of players, you always ask for Player one's name. Regardless of the number of players, you always ask if there is a computer player. If you moved these questions out of the if-statements, you don't need to repeat them.

print('1 or 2 Player?')
playerAmount = int(input())

print('Computer Player? [Y/N]')
c1 = input()

print('Player one enter your name')
p1 = input()

if playerAmount == 2:
    print('Player two enter your name')
    p2 = input()

Note that you are asking for a Y or a N for the computer player, but later check if c1 is a y. This is dangerous. It would be wise to converting c1 to a lower case, if you know how to do that, or ask 'Computer Player? [y/n]' otherwise.


The main portion of your program reads:

if playerAmount == 1 and c1 != "y":
    # Player 1 game
    # Output score
elif playerAmount == 2 and c1 != "y":
    # Player 1 game
    # Player 2 game
    # Output score
elif c1 == "y" and playerAmount == 1:
    # Player 1 game
    # Computer game
    # Output score
elif c1 == "y" and playerAmount == 2:
    # Player 1 game
    # Player 2 game
    # Computer game
    # Output score
else:
    print('Thanks for playing')

Note the following:

  • "Player 1 game" appears in 4 branches
  • "Player 2 game" appears in 2 branches
  • "Computer game" appears in 2 branches
  • "Output score" appears in 4 branches
  • "Thanks for playing" is only printed if no games are played.

We can do much better by organizing the code as follows:

# Player 1 game

if playerAmount == 2:
    # Player 2 game

if c1 == "y":
    # Computer game

# Output score

print('Thanks for playing')

This removes 3 copies of the "Player 1 game" code, 1 copy of the "Player 2 game" code, 1 copy of the "Computer game" code.

However, "Output score" will become more complex, as we will need to distinguish between the combinations of players when we compute who wins. Still, it looks like a significant reduction in code is possible.


The guessing code for player 1 follows. The code for player two and the computer is similar.

p1Guess = int(input())
while p1Guess != numToGuess:
    p1NumOfGuess += 1

    if p1Guess < numToGuess:
        print("Guess is to Low!""\n""Guess Again")
        p1Guess = int(input())

    elif p1Guess > numToGuess:
        print("Guess is to High!""\n""Guess Again")
        p1Guess = int(input())

print("Congrats", p1Guess, "is correct!")

Here, I see 3 places where a guess is input p1Guess = int(input()). The first is outside the loop, which is why you have the awkward initialization to 1 at the start of the code. We can improve this code by starting the count at 0, inputting the guess at exactly one point in the code, and incrementing the count when the guess occurs.

numToGuess = random.randint(1, 100)
print(p1, "Guess a Number between 1 and 100")

p1NumOfGuess = 0

while True:
    p1Guess = int(input())
    p1NumOfGuess += 1

    if p1Guess < numToGuess:
        print("Guess is too Low!\nGuess Again")

    elif p1Guess > numToGuess:
        print("Guess is too High!\nGuess Again")

    else:
        print("Congrats", p1Guess, "is correct!")
        break

Player two game code would look similar, but contained in an if playerAmount == 2: block.


The computer game code would also look similar, but with the preLow and prevHigh variables.

if c1 == 'y':

    numToGuess = random.randint(1, 100)

    print("Computer is guessing")

    prevLow = 1
    prevHigh = 100
    c1NumOfGuess = 0

    while True:
        c1Guess = random.randint(prevLow, prevHigh)
        c1NumOfGuess += 1
        time.sleep(1.25)

        if c1Guess < numToGuess:
            print(c1Guess, "\nGuess is too Low!\nGuess Again")
            prevLow = c1Guess + 1    # Slight boost to the computer AI

        elif c1Guess > numToGuess:
            print(c1Guess, "\nGuess is too High!\nGuess Again")
            prevHigh = c1Guess - 1   # Slight boost to the computer AI

        else:
            print("Congrats", c1Guess, "is correct!")
            break

Scoring. First, you are printing out the number of guesses for each player. Like the above, you always print out player one's guess count, but only need to print out player two's and the computer's if they actually played.

print(p1, 'took', p1NumOfGuess, 'guesses to guess correctly!')

if playerAmount == 2:
    print(p2, 'took', p2NumOfGuess, 'guesses to guess correctly!')

if c1 == "y":
    print('Computer took', c1NumOfGuess, 'guesses to guess correctly!')

Now, you only need to determine who won, but only if there were multiple players. We'll cheat a little here, and set the number of guesses of players who didn't play to a large number:

if playerAmount == 2  or  c1 == 'y':

    # Give unrealistically large guess counts to players who didn't play.

    if playerAmount != 2:
        p2NumOfGuess = 99999
    if c1 != 'y':
        c1NumOfGuess = 99999


    # Determine winner among all 3 players.

    if p1NumOfGuess < p2NumOfGuess and p1NumOfGuess < c1NumOfGuess:
        print(p1, 'WINS!')

    elif p2NumOfGuess < p1NumOfGuess and p2NumOfGuess < c1NumOfGuess:
        print(p2, 'WINS!')

    elif c1NumOfGuess < p1NumOfGuess and c1NumOfGuess < p2NumOfGuess:
        print('Computer WINS!')

    else:
        print('Tied Game!')

Use functions to organize your code, and reduce code duplication:

Add the following function definition near the top of the file:

def playGame(player_name):

    numToGuess = random.randint(1, 100)
    print(player_name, "Guess a Number between 1 and 100")

    numOfGuesses = 0

    while True:
        guess = int(input())
        numOfGuesses += 1

        if guess < numToGuess:
            print("Guess is too Low!\nGuess Again")

        elif guess > numToGuess:
            print("Guess is too High!\nGuess Again")

        else:
            print("Congrats", guess, "is correct!")
            break

    return numOfGuesses

Then, you can replace the Player one game and Player two game code with:

p1NumOfGuess = playGame(p1)

if playerAmount == 2:
    p2NumOfGuess = playGame(p2)
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the time and help with this. Implementing all of this did reduce the code by about 100 lines. I did use c1 = c1.casefold() to solve the Y/N problem. I have been trying to get my head around functions but am struggling there. I'm trying to come up with a function that will keep an alphabetic input from breaking the code at the moment. \$\endgroup\$
    – Ineffable
    Commented Nov 20, 2018 at 14:09
  • \$\begingroup\$ Struggling between writing a function for you (which we don’t do in Code Review), and sending you over to Stack Overflow with instructions to post your attempt with a function for them to help you debug/fix. In the end, I decided it was simpler and perhaps more helpful to give you an “example function” to show you how to organize redundant code into a function called from two places. \$\endgroup\$
    – AJNeufeld
    Commented Nov 20, 2018 at 19:27
1
\$\begingroup\$

Welcome to Code Review. This is not bad code, considering you identify yourself as a beginner. The first thing (perhaps before this is even reviewable) is to learn how to write functions. There is a lot of repeated code, and you can condense it by factoring it into methods that can execute the code many times. Have a read through this:

https://www.w3schools.com/python/python_functions.asp

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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