HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberthedieuserrollthatsidesentersaccording

Problem

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?

Solution

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).

Code Snippets

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)

Context

StackExchange Code Review Q#37163, answer score: 3

Revisions (0)

No revisions yet.