6
\$\begingroup\$

How can I make this better?

import random

def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6) + random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return dice

def checkRoll(dice):
    if dice == 7 or dice == 11:
        print("You win")
        playAgain()
    elif dice == 2 or dice == 3 or dice == 12:
        print("You lose")
        playAgain()
    else:
        print("Time to try to roll the point")
        reRoll(dice)

def reRoll(point):
    dice = roll()
    if dice == point:
        print("You win")
        playAgain()
    elif dice == 7:
        print("You lose")
        playAgain()
    else:
        reRoll(point)
def play():
    print("Do you wanna play Craps? Y/n ")
    response = input(">>> ").lower()
    if response == "y":
        checkRoll(roll())
    else:
        input("ENTER to quit")

def playAgain():
    response = input("Play Again? Y/n >>>").lower()
    if response == "y":
        checkRoll(roll())
    else:
        input("ENTER to quit")


play()
\$\endgroup\$
3
\$\begingroup\$

PEP8

1.Python uses underscore as naming separator in function and variable names, see PEP8

2.Two blank lines are used to separate function/classes

Improvements.

To make your module reusable you should use if __name__ == '__main__' condtion before invoking your main function.

So instead of calling play() you should do:

if __name__ == '__main__':
    play()

Your play_again and play function got difference only in print statements, so you can just combine them into a single function.

def start_game(first_run=False):
    if first_run:
        print("Do you wanna play Craps? Y/n ")
        response = input(">>> ").lower()
    else:
        response = input("Play Again? Y/n >>>").lower()
    if response == "y":
        check_roll(roll())
    else:
        input("ENTER to quit")

Note

I've renamed function into start_game since I find play a bit confusing. But you can keep your name if you want.

Now if you look at your check_roll and reroll function they are also the same except for win and lose conditions, so you can also combine them into a single one like this:

def check_roll(dice, win_conditions=(7, 11), lose_conditions=(2, 3, 12)):
    if dice in win_conditions:
        print("You win")
        start_game()
    elif dice in lose_conditions:
        print("You lose")
        start_game()
    else:
        print("Time to try to roll the point")
        check_roll(roll(), (dice,), (7,))

Also, I did small improvement to roll() function so now you can see values of dice you've rolled:

def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6), random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return sum(dice)

So in the end, what we have is this:

import random


def roll():
    input("Press Enter to roll")
    dice = random.randint(1, 6), random.randint(1, 6)
    print("You rolled a {}" .format(dice))
    return sum(dice)


def check_roll(dice, win_conditions=(7, 11), lose_conditions=(2, 3, 12)):
    if dice in win_conditions:
        print("You win")
        start_game()
    elif dice in lose_conditions:
        print("You lose")
        start_game()
    else:
        print("Time to try to roll the point")
        check_roll(roll(), (dice,), (7,))


def start_game(first_run=False):
    if first_run:
        print("Do you wanna play Craps? Y/n ")
        response = input(">>> ")
    else:
        response = input("Play Again? Y/n >>>")
    if response.lower() == "y":
        check_roll(roll())
    else:
        input("ENTER to quit")


if __name__ == '__main__':
    start_game(True)
| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

Your code is quite good, it's testable. But it may abruptly break as it uses recursion.

Spam y and enter for a reasonable period of time and you should get:

RecursionError: maximum recursion depth exceeded

To fix this, change from using recursion to a loop. To do this merge; play, reRoll, and playAgain. And; checkRoll, and reRoll, should return, rather than call any of these functions.

Firstly merge reRoll with checkRoll, to do this you want to use a while loop, remove the else in reRoll and replace all playAgain calls with return. This can get something like:

def checkRoll(point):
    if point == 7 or point == 11:
        print("You win")
        return
    elif point == 2 or point == 3 or point == 12:
        print("You lose")
        return
    else:
        print("Time to try to roll the point")
        while True:
            dice = roll()
            if dice == point:
                print("You win")
                return
            elif dice == 7:
                print("You lose")
                return

After this merge play and playAgain. To do this use a while loop and call the above checkRoll with the code to play again there. This can get:

def play():
    print("Do you wanna play Craps? Y/n ")
    while True:
        response = input(">>> ").lower()
        if response != "y":
            break
        checkRoll(roll())
        print("Play Again? Y/n")
    input("ENTER to quit")

After removing the recursion there are still a couple of ways I'd change the code:

  1. You can check if an item is in a list, rather than using or. 'a' in ['a', 'b'].
  2. The code should default empty input to y if when using Y/n. To do this compare to n rather than y.
  3. I'd move the prints to the main.
  4. I'd move the rolling line in roll, to it's own function. So if you need to roll without user input at a later date you can.

All this can get: (The comments are only there to show what I changed)

import random

# 4
def roll_():
    return random.randint(1, 6) + random.randint(1, 6)

def roll():
    input("Press Enter to roll")
    dice = roll_()
    print("You rolled a {}" .format(dice))
    return dice

def checkRoll(dice):
    if dice in [7, 11]: # 1
        return True # 3
    elif dice in [2, 3, 12]: # 1
        return False # 3
    else:
        print("Time to try to roll the point")
        while True:
            dice = roll()
            if dice == point:
                return True # 3
            elif dice == 7:
                return False # 3

def play():
    print("Do you wanna play Craps? Y/n ")
    while True:
        response = input(">>> ").lower()
        if response == "n": # 2
            break
        if checkRoll(roll()):
            print("you win") # 3
        else:
            print("you lose") # 3

    input("ENTER to quit")

play()
| improve this answer | |
\$\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.