11
\$\begingroup\$

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[0], completed[1], completed[2], completed[3], completed[4], completed[5], completed[6]) print "9s: %d \n10s: %d \n11s: %d \n12s: %d""" % (completed[7], completed[8], completed[9], completed[10]) raw_input("") roll() def storetotal(): if total == 2: completed[0] += 1 elif total == 3: completed[1] += 1 elif total == 4: completed[2] += 1 elif total == 5: completed[3] += 1 elif total == 6: completed[4] += 1 elif total == 7: completed[5] += 1 elif total == 8: completed[6] += 1 elif total == 9: completed[7] += 1 elif total == 10: completed[8] += 1 elif total == 11: completed[9] += 1 else: completed[10] += 1 roll()
\$\endgroup\$
1
  • \$\begingroup\$ I tried all the above codes but it shows invalid syntax T.T \$\endgroup\$ Commented May 12, 2016 at 17:56

2 Answers 2

Reset to default
15
\$\begingroup\$

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[0], completed[1], completed[2], completed[3], completed[4], completed[5], completed[6]) print "9s: %d \n10s: %d \n11s: %d \n12s: %d""" % (completed[7], completed[8], completed[9], completed[10])

Indentation here is completely messed up.

raw_input("") roll() def storetotal():

PEP8: store_total.

if total == 2: completed[0] += 1 elif total == 3: completed[1] += 1 elif total == 4: completed[2] += 1 elif total == 5: completed[3] += 1 elif total == 6: completed[4] += 1 elif total == 7: completed[5] += 1 elif total == 8: completed[6] += 1 elif total == 9: completed[7] += 1 elif total == 10: completed[8] += 1 elif total == 11: completed[9] += 1 else: completed[10] += 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()
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Additional suggestion : stats = [0] * 10 \$\endgroup\$ Commented Apr 4, 2014 at 23:14
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ Commented Apr 4, 2014 at 23:22
  • \$\begingroup\$ Check highest voted answer: stackoverflow.com/questions/119562/… ... \$\endgroup\$ Commented Apr 5, 2014 at 1:25
  • \$\begingroup\$ 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? \$\endgroup\$ Commented Apr 5, 2014 at 2:13
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ Commented Apr 5, 2014 at 2:54
8
\$\begingroup\$

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()
\$\endgroup\$
1
  • 1
    \$\begingroup\$ 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. \$\endgroup\$ Commented Apr 4, 2014 at 23:01

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.