Roll a die according to the number of sides that the user enters

Question

I have created a program which rolls a die according to the number of sides that the user enters:

def var():

    import random

    dicesides = int(input("Please enter the amount of sides you want the dice that is being thrown to have. The dice sides available are: 4, 6 and 12: "))
    script(dicesides, random)

def script(dicesides, random):

    if dicesides == 4:
        dice4 = int(random.randrange(1, dicesides))
        print(dicesides, " sided dice, score", dice4)

    elif dicesides == 6:
        dice6 = int(random.randrange(1, dicesides))
        print(dicesides, " sided dice, score", dice6)

    elif dicesides == 12:
        dice12 = int(random.randrange(1, dicesides))
        print(dicesides, " sided dice, score", dice12)

    elif dicesides != 4 or dicesides != 6 or dicesides != 12:
        print("That number is invalid. Please try again.")
        var()

    repeat = str(input("Repeat? Simply put yes or no : "))

    if repeat == "yes":
        var()
    else:
        quit()

var()

Is there a way to shorten this?

Answer 1

Your variables dice4. dice6 and dice12 are essentially the same variables, so let's give them the same name. It now happens that three of the cases are identical, and can be shortened to:

if dicesides != 4 or dicesides != 6 or dicesides != 12:
    print("That number is invalid. Please try again.")
    var()
else:
    score = int(random.randrange(1, dicesides))
    print(dicesides, " sided dice, score", score)

The test for allowed numbers of sides can be shortened to dicesides not in [4, 6, 12], which more clearly shows what you are trying to express. This would allow you to include the number of sides as an optional parameter to the function – some people use 20-sided dices a lot. And actually, there is no reason why you would constrain the user to a certain number of sides anyway.

It is not clear to me what advantage recursion via the var method has over an ordinary loop. One disadvantage is that an avid player could theoretically overflow the stack….

It is also not clear why random is a parameter to the script.

You are using random.randrange wrong. The help() output on my system gives me:

Choose a random item from range(start, stop[, step]).

This fixes the problem with randint() which includes the endpoint; in Python this is usually not what you want.

This means that rolling a 6-sided die, you will invoke random.randrange(1, 6), which returns a random int in the range [1, 6) – this excludes the number 6, and you actually pick from 1, 2, 3, 4, 5. Fix this either by 1 + random.randrange(0, dicesides) or preferably random.randint(1, dicesides).

Answer 2

Absolutely! If you replace dice4, dice6 and dice12 with rolled, you'll see they all have the same code. At that point, just gate the shared code with something like if dicesides in [4, 6, 12]:, and change your final elif to an else.

Other comments:

• I would suggest not passing random to script() (which itself should have a better name, such as roll).
• The way you loop uses a technique that looks a lot like recursion. However your approach is not using this in any meaningful fashion. I would probably move the part of your code from repeat = str(input(... into var() (which itself should either have a more meaningful name, or at least a common name like main), and put this in a while loop.

Putting these together:

import random

def main():
    repeat = "yes"
    while repeat == "yes":
        dicesides = ...
        roll(dicesides)
        repeat = ...

def roll(dicesides):
    if dicesides in [4, 6, 12]:
        rolled = int(random.randrange(1, dicesides)) # but see amon's notes
        print(dicesides, " sided dice, score", rolled)
    else:
        print("That number is invalid. Please try again.")

main()

Note that this does result in a slight change on invalid input - it now asks you if you want to repeat.