# Dice-rolling simulator

I know there is a better way to store data. What is the most concise way to simplify this script?

from random import randint
from sys import exit
import os

os.system('clear')
print "Welcome to the dice rolling simulator!"
raw_input("Press enter to begin.")

total = 0

completed = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

def roll():
os.system('clear')
die1 = randint(1, 6)
die2 = randint(1, 6)
global total
total = die1 + die2
storetotal()
print "Die 1: %d \nDie 2: %d \nTotal: %d." % (die1, die2, total)
print "\n\nRoll again?"
roll_again = raw_input("Press enter to roll again, type 'stats' to view scores, or 'quit' to exit.\n> ")
if roll_again == "":
roll()
elif roll_again == "stats":
stats()
elif roll_again == "quit":
exit(0)
else:
print "I don't know what that means so you get to roll again."
raw_input("> ")
roll()

def stats():
global total
print "2s: %d \n3s: %d \n4s: %d \n5s: %d \n6s: %d \n7s: %d \n8s: %d" % (completed,
completed, completed, completed,
completed, completed, completed)
print "9s: %d \n10s: %d \n11s: %d \n12s: %d""" % (completed, completed,
completed, completed)
raw_input("")
roll()

def storetotal():
if total == 2:
completed += 1
elif total == 3:
completed += 1
elif total == 4:
completed += 1
elif total == 5:
completed += 1
elif total == 6:
completed += 1
elif total == 7:
completed += 1
elif total == 8:
completed += 1
elif total == 9:
completed += 1
elif total == 10:
completed += 1
elif total == 11:
completed += 1
else:
completed += 1

roll()

• I tried all the above codes but it shows invalid syntax T.T – user105283 May 12 '16 at 17:56

# Line by line analysis

Use four spaces to indent python code. Never use tabs.

from random import randint
from sys import exit
import os


It is a good practice to do top-level imports (without importing module members). This way it's easier to understand what package the methods come from.

os.system('clear')
print "Welcome to the dice rolling simulator!"
raw_input("Press enter to begin.")

total = 0

completed = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]


Use list comprehensions.

def roll():


Roll method is confusing. You expect it to roll a die, but it processes iterations of the game. A while loop would be more explicit here.

os.system('clear')
die1 = randint(1, 6)
die2 = randint(1, 6)
global total


Global variables are no-no. Never.

total = die1 + die2
storetotal()
print "Die 1: %d \nDie 2: %d \nTotal: %d." % (die1, die2, total)
print "\n\nRoll again?"
roll_again = raw_input("Press enter to roll again, type 'stats' to view scores, or 'quit' to exit.\n> ")


You're exceeding recommended 80 column length limit.

if roll_again == "":
roll()


See how I changed this block in a code below.

elif roll_again == "stats":
stats()
elif roll_again == "quit":
exit(0)


Return from the method is sufficient here.

else:
print "I don't know what that means so you get to roll again."
raw_input("> ")
roll()

def stats():
global total
print "2s: %d \n3s: %d \n4s: %d \n5s: %d \n6s: %d \n7s: %d \n8s: %d" % (completed,                                                                     completed, completed, completed,
completed, completed, completed)
print "9s: %d \n10s: %d \n11s: %d \n12s: %d""" % (completed, completed,
completed, completed)


Indentation here is completely messed up.

raw_input("")
roll()

def storetotal():


PEP8: store_total.

if total == 2:
completed += 1
elif total == 3:
completed += 1
elif total == 4:
completed += 1
elif total == 5:
completed += 1
elif total == 6:
completed += 1
elif total == 7:
completed += 1
elif total == 8:
completed += 1
elif total == 9:
completed += 1
elif total == 10:
completed += 1
elif total == 11:
completed += 1
else:
completed += 1

roll()


# Improved code

I simplified your roll method to roll2d6 and moved all the other code into main.

import random
import os

def roll2d6():
die1 = random.randint(1, 6)
die2 = random.randint(1, 6)
return die1 + die2

def display_stats(stats):
for i, result in enumerate(stats):
print "%ds: %d" % (i + 2, result)

def update_stats(total, stats):
stats[total - 2] += 1
return stats

def main():
stats = [0 for _ in xrange(10)]

os.system('clear')
print "Welcome to the dice rolling simulator!"
raw_input("Press enter to begin.")
while True:
os.system('clear')
total = roll2d6()
stats = update_stats(total, stats)
print "Die 1: %d \nDie 2: %d \nTotal: %d." % (die1, die2, total)
print "\n\nRoll again?"
roll_again = raw_input((
"Press enter to roll again, type 'stats' to view scores, "
"or 'quit' to exit.\n> "))
if roll_again.lower() == "stats":
display_stats(stats)
elif roll_again.lower() == "quit":
break
elif roll_again.lower() != "":
print "I don't know what that means so you get to roll again."

if __name__ == "__main__":
main()

• Additional suggestion : stats =  * 10 – Josay Apr 4 '14 at 23:14
• Also : call lower() on the result of raw_input() before storing in roll_again / write elif roll_again.lower() == "": continue to have all conditions written in a positive way - it makes the code longer but also clearer imho. – Josay Apr 4 '14 at 23:22
• Check highest voted answer: stackoverflow.com/questions/119562/… ... – David Mulder Apr 5 '14 at 1:25
• display_stats(stats) would be cleaner with enumerate(stats, start=2) rather than i + 2 on every iteration and update_stats(total, stats) is just a waste of space, it is only called once and is basically a one-liner so why not inline it? – Adam Apr 5 '14 at 2:13
• There's a coding style that any logical selfcontained subunit should be def'd so it can be reused elsewhere. Space is cheap, and you won't notice any speed difference. – Snowbody Apr 5 '14 at 2:54

So, first, it's all a bit confusing when the indentation is messed up (which is important in Python). You should first get that in order.

Also globals are usually a bad idea, and they're not necessary here. It also appears that, even properly executed, you've built this program as an infinite recursion (which is very inefficient). Instead you should just have your code in a loop implemented as a finite state machine (one state for rolling, another for stats, and an exit-state for quitting.

Also python uses underscore_naming so storetotal should be store_total (or better yet update_total). Also storetotal can be simplified greatly to a single line:

completed[total - 2] += 1


And the stats printing lines can be similarly simplified:

for i in xrange(0, 11):
print '%ds: %d' % (i + 2, completed[i])


From a style perspective, Python also uses single-quotes, not double-quotes.

The resultant code should look something like this:

from random import randint
from sys import exit
import os

def handle_roll(total_counts):
die1, die2 = randint(1, 6), randint(1, 6)
total = die1 + die2
total_counts[total - 2] += 1

print 'Die 1: %d \nDie 2: %d \nTotal: %d.' % (die1, die2, total)
print '\n\nRoll again?'
response = raw_input('Press enter to roll again, type "stats" to view scores, or "quit" to exit.\n> ').lower()
if response == '':
return handle_roll
elif response == 'stats':
return handle_stats
elif response == 'quit':
return None

return handle_unknown

def handle_stats(total_counts):
for i in xrange(0, 11):
print '%ds: %d' % (i + 2, total_counts[i])
raw_input('')
return handle_roll

def handle_unknown(total_counts):
print 'I don\'t know what that means so you get to roll again.'
raw_input('')
return handle_roll

def main():
os.system('clear')
print 'Welcome to the dice rolling simulator!'
raw_input('Press enter to begin.')

total_counts = [0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]
state = handle_roll
while state != None:
os.system('clear')
state = state(total_counts)

if __name__ == "__main__":
main()

• Using single or double quotes in python is a matter of preference. Also, PEP 257 enforces using double quotes for docstrings. Agree with the rest. – Ruslan Osipov Apr 4 '14 at 23:01