# Interactive dice simulator

After finishing my course on CodeAcademy, I wanted to write a program. Please provide opinions.

from random import randint
import sys, time, os

def clear(): #Usefull for clearing the terminal, should be cross-platform
os.system('cls' if os.name=='nt' else 'clear')

def initialize_variables(): #Self Described
global count, start, count_pos, initi_pos
count = 0
start = ""
count_pos = 2
initi_pos = 0

def open_file(): #Opens the options' file
global options_file
options_file = open("options.txt", "a+")

def close_file(): #Closes the option's file
global options_file
options_file.close()
#return options_file.closed #Debugging purposes

def initial_setup(): #
global options_file
global count_pos, initi_pos
options_file.seek(initi_pos, 0)
if options_file.read(1) == "": #Check the first value of the file, to determine if the user is running the Dice Simulator (DS from now on)
options_file.write("0")  #If no value found, set it at default "0"

options_file.seek(initi_pos, 0) #Reposition cursor
if (options_file.read(1) == "0") : #If default value found, the user's running the program for the first time.
print "Welcome to the dice simulator"
print "It seems like you are here for the first time."
options_file.seek(count_pos, 0) #Reposition cursor
options_file.write(str(raw_input("After how many times you want to change the dice? "))+ "\n")
print "Done!",
for i in range(1,4):
time.sleep(0.5)
print ".",

options_file.seek(initi_pos, 0) #Reposition cursor to initi value
options_file.write("1") #Change it to our new default value
time.sleep(2)
clear() #Clear the window
else:
pass

def change_options():
global options_file
count_2file = int(raw_input("After how many rolls will you change the dice? "))
options_file.seek(count_pos,0)
options_file.write(str(count_2file))
clear()

print "Dice Rolling Simulator |"
print "-----------------------/"
print "1. Start rolling"
print "2. Change options"
print "3. Quit"
choice = int(raw_input("Your selection is: "))

if choice == 1: #If first choice is selected
game() #Start the game
elif choice == 2: #If second choice is selected
change_options() #Change the options
elif choice == 3: #If third choice is selected
print "Application will now exit."
time.sleep(3)
sys.exit() #Kill the app.
else: #INVALID INPUT
print "Invalid input. Error code: 0x001" #Print the appropriate message
print "Application will now exit."
time.sleep(3)
sys.exit() #Kill the app

def game():
global start
x = int(raw_input("The size of the dice(s) is: "))
y = int(raw_input("How many times will you roll it? "))
start = raw_input("Roll the dice (y/n)? ")

while True:
if start == "y":
clear()
print roll_dice(x,y)
start = roll_again()
global count
count += 1
promtp_change(count)
elif start == "n":
clear() #Clear screan
print "Application will now exit."
time.sleep(2)
close_file() #Before exit make sure to close the file
sys.exit()
elif start != "y" and start != "n":
clear()
print "Invalid input. Error code: 0x001"
print "Application will now exit."
time.sleep(2)
close_file() #Before exit make sure to close the file
sys.exit()

###-###-###-###-###-###-###-###-###
def roll_dice(sides, num_rolls):
total_rolls = 0
roll_result = 0
result_sum = 0
for i in range(0, num_rolls):
roll_result = randint(1, sides)
print "Die #%d rolled %d" % (i+1, roll_result)
result_sum += roll_result
total_rolls += 1
print "Total of %d dice rolls is: %d" % (num_rolls, result_sum)
return None

def roll_again():
global start
start = raw_input("Roll again? ")
return start

def promtp_change(cnt):
global options_file, count_pos
options_file.seek(count_pos, 0)
pref_val_cnt = options_file.read(1) #The user's saved value for 'count'
if int(cnt) == int(pref_val_cnt): #If they are the same, prompt to change dice
temp_var_a = raw_input("Change dice? ")
if temp_var_a == "y": #If yes
clear()
game() #Restart the game, by calling it again
else:
pass #Elsewise continue

###*###*###*###*###*###*###*###*###
def main():
initialize_variables()
open_file()
initial_setup()
###*###*###*###*###*###*###*###*###

#----------------------Drop zone----------------------#
main()
#-----------------------ERRORS-EXPLAINED-----------------------#
# 0x001 : User's input was not expected.
#         Possible reasons, most common:
#                   *Value not in possible choices
#                   *Expected str, input is int or vice-versa


def initialize_variables(): #Self Described
global count, start, count_pos, initi_pos
count = 0
start = ""
count_pos = 2
initi_pos = 0

• Do not use global variables, never. Functions are made to process input and give output. Your functions do not take input parameter. Change your code so that the functions will take input and there will not be global variables (or few of them).

• You have a nonsense comment: #Self Described. Comments like this one should be avoided because they just confuse the reader.

def open_file(): #Opens the options' file
global options_file
options_file = open("options.txt", "a+")

def close_file(): #Closes the option's file
global options_file
options_file.close()
#return options_file.closed #Debugging purposes


These functions are meaningless because you can use:

with open('options.txt') as f:
#do something


that will also handle closing automatically.

Error code: 0x001


What? You should print a human readable error message.

print "Dice Rolling Simulator |"
print "-----------------------/"
print "1. Start rolling"
print "2. Change options"
print "3. Quit"


instead for better clarity you should print a multiline message declared at the start of the file:

MENU = """Dice Rolling Simulator |"
"-----------------------/"
"1. Start rolling"
"2. Change options"
"3. Quit"""

# other code


• I'm not sure I agree about avoiding multiple lines of print; what matters is separating UI from logic. – Veedrac Jan 4 '15 at 17:27
• @Veedrac It is not about avoiding many print but about using the most constants possible. Anyway this is a minor point and as you say, it is much more important to learn to separate UI from logic. – Caridorc Jan 4 '15 at 17:41

Your code has no major issues, just a few problems to do with convention.

Documentation
Most of your functions are like this:

def clear(): #Usefull for clearing the terminal, should be cross-platform


When usually, you would use docstrings, usually saying what arguments the function takes and what it returns:

def clear():
"""Clears stdout, returns None"""


(The 'terminal' you reference is probably 'stdout')

Global Variables
They may seem useful at first, but should generally be avoided. A way to avoid this is passing variables into functions, for example:

def initial_setup(options_file, count_pos, initi_pos):
stuff()


But how would you store those variables? You could use ambiguously indexed lists, or a dictionary, or you could use a class, except with a program this simple a dictionary is probably the way to go.

Error Messages
You print an error message with a hexadecimal value. This is not human readable. You should instead:

choice = raw_input("Your selection is: ")
try:
choice = int(choice)
except ValueError:
raise TypeError("Input must be of type int, not", type(choice).__name__)
if choice not in (1, 2, 3):
raise ValueError("Your selection must be either 1, 2 or 3")


Recursion
When your program ends, it asks you if you want to restart it. To restart, it calls the menu() function again. This works in your situation, but it is good to know the dangers. Python has a max recursion depth, which means this will only work some times until it reaches max recursion depth and crashes. You could prevent this by putting the whole contents of the main() fuction in a while loop.

• Incredibly the documentation bit was exactly what I was writing in my answer... but you were faster posting. – Caridorc Jan 4 '15 at 17:13

I think that if you are up to coming to CR you won't badmouth PEP-8. If you do not know what it is, read it.

Imports should be on separated lines.

Two empty lines before a function declaration (one if it is inside a class).

#return options_file.closed #Debugging purposes


Good. But this should be production code, removing debugging code (not just by commenting out). Any serious development (you called this a program) should be done under VCS, in such environments you just delete code, don't comment it out.

###*###*###*###*###*###*###*###*###


Is this a fence to protect a function from the other functions?

#-----------------------ERRORS-EXPLAINED-----------------------#
# 0x001 : User's input was not expected.
#         Possible reasons, most common:
#                   *Value not in possible choices
#                   *Expected str, input is int or vice-versa


Good, this way when the user sees 0x001 in the screen he/she e-mails you asking what the error is and then you either tell them how to edit the file or read it for them.

Short Python programs usually are "doable" without even a single global variable. Try it.

Someone above suggested classes. Don't go that way. Python classes are slow and fat and you just want to roll a dice, it is not really necessary to have classes here.

Last but not least, try to name functions with verbs or verbal phrases. It makes more sense to kill_foo_bar() than foo_bar_death().

Your imports break PEP 8. They should be something like

import os
import sys
import time

from random import randint


You often comment your functions like

def x(): # does a thing


Not only do I personally dislike end-of-line comments, these should be doc comments

def x():
"""Does a thing."""


Also, don't make comments like #Self Described.

You do screen constrol mostly by just running clear at opportune moments. This would be improved if you used a library built for this. Normally one would mention something terribly complicated like ncurses but I shall mention blessings, easily the nicest way to mess with the screen.

You are using global variables. Don't. A simple solution is just to pass these around as arguments. Note that as I made this change it became apparent that initialize_variables wasn't needed since it mostly initializes global constants.

#Check the first value of the file, to determine if the user is running the Dice Simulator (DS from now on)


Your if here is formatted oddly:

if (options_file.read(1) == "0") :


You can just do

if options_file.read(1) == "0":


This loop won't work like you expect:

for i in range(1,4):
time.sleep(0.5)
print ".",


The terminal is buffered so you need to flush is between prints.

It seems like you're using options_file as an ad-hoc data store. You should look into structured approaches like JSON or databases. The easiest in this case, perhaps, would be shelve. This allows sweeping simplifications to the code.

clear() #Clear the window


are useless. It's just repeating what's already there. Remove them.

Don't do decorations like:

 ###-###-###-###-###-###-###-###-###


There's no need to return None from roll_dice, and no need to print the return value (which is None).

I don't get what this is:

#----------------------Drop zone----------------------#
main()
#-----------------------ERRORS-EXPLAINED-----------------------#
# 0x001 : User's input was not expected.
#         Possible reasons, most common:
#                   *Value not in possible choices
#                   *Expected str, input is int or vice-versa


I suggest you remove the comments.

You don't need to write

else:
pass


You should handle your files with a with statement, not manual closing. Unfortunately you're using Python 2, so you'll need contextlib.closing. open_file doesn't really do anything, so remove and inline it.

x = int(raw_input("The size of the dice(s) is: "))
y = int(raw_input("How many times will you roll it? "))


Why x and y?

You are using haphazard recursion to generate loops. Instead, define suitable functions to guide your loops and make functions self-contained where possible. For example, instead of using # Restart the game by calling it again, have an outer loop that calls promtp_change after the game has ended to allow it to restart. This should remove the need to call sys.exit from the functions (you shouldn't be doing that). Because the game seems a bit underspecified, I wasn't able to bring all behaviours across: what exactly is promtp_change meant to do? It seems to do nothing.

game has duplicated logic in the ifs. Move it outside of the ifs. This is also duplicated in menu, so extract it into a function.

I would move to new-style formatting ("{}".format instead of "%s" %).

In roll_dice, you don't need total_rolls since it's just num_rolls. You shouldn't give a default to roll_result; it just hides bugs. You don't need a 0 argument to range. In fact, I would write it as

rolls = [random.randint(1, sides) for _ in range(num_rolls)]

for i, roll in enumerate(rolls, 1):
print "Die #{} rolled {}".format(i, roll)

print "Total of {} dice rolls is: {}".format(num_rolls, sum(rolls))


This all gives a much cleaner control flow and execution model, and the much less buggy code:

import random
import shelve
import sys
import time

from contextlib import closing

from blessings import Terminal

def initial_setup(options):
if not options.get("has_run", False):
options["has_run"] = True

print "Welcome to the dice simulator"
print "It seems like you are here for the first time."
set_options(options)
print "Done!",

for i in range(1, 4):
time.sleep(0.5)
print ".",
sys.stdout.flush()
time.sleep(2)

def set_options(options):
options["count"] = int(raw_input("After how many rolls will you change the dice? "))

while True:
print "Dice Rolling Simulator |"
print "-----------------------/"
print "1. Start rolling"
print "2. Change options"
print "3. Quit"
choice = int(raw_input("Your selection is: "))

if choice == 1:
game(term, options)
return

elif choice == 2:
set_options(options)
print(term.clear())

else:
return

def game(term, options):
while True:
die_size = int(raw_input("The size of the dice(s) is: "))
die_count = int(raw_input("How many times will you roll it? "))
start = raw_input("Roll the dice (y/n)? ")

for count in range(options["count"]):
print(term.clear())

if start == "y":
roll_dice(die_size, die_count)
start = raw_input("Roll again? ")

else:
return

# What prompt_change would do... if it did something
raw_input("Change dice? ")

if is_clean:
print "Application will now exit."
else:
print "Invalid input. Error code: 0x001"
print "Application will now exit."

time.sleep(sleep_for)

def roll_dice(sides, num_rolls):
rolls = [random.randint(1, sides) for _ in range(num_rolls)]

for i, roll in enumerate(rolls, 1):
print "Die #{} rolled {}".format(i, roll)

print "Total of {} dice rolls is: {}".format(num_rolls, sum(rolls))

def main():
term = Terminal()

with closing(shelve.open("options.txt")) as options:
initial_setup(options)
print(term.clear())

main()


However, this hasn't separated logic from UI (output), which is another important aspect of code quality. It also doesn't deal with invalid input (such as non-integers being passed to int).

• 'prompt_change'. With this fucntion, I ask the user if he wants to change dices after a number of rolls. For each roll, I add 1 to the variable 'count'. With calling 'prompt_change', I compare 'count' with the value on 'nth' bit of the file 'options.txt'. If they are equal, the user can change dice. Else, nothing happens. I hope I made it clear to you. Note that I've deleted every single comment, since they weren't commenting in any usefull way. – TheAngryAvocado Jan 4 '15 at 22:06

Just fyi .. There nothing wrong in doing main(), but another preferred way is

if __name__ =='__main__':
main()
.. OR  ..
initialize_variables()
open_file()
initial_setup()