patternpythonMinor
Roll a die according to the number of sides that the user enters
Viewed 0 times
numberthedieuserrollthatsidesentersaccording
Problem
I have created a program which rolls a die according to the number of sides that the user enters:
Is there a way to shorten this?
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
The test for allowed numbers of sides can be shortened to
It is not clear to me what advantage recursion via the
It is also not clear why
You are using
Choose a random item from
This fixes the problem with
This means that rolling a 6-sided die, you will invoke
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.