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

Script to determine the outcome(s) of any number of dice rolls

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

Problem

I think this code has far to many if else clauses and not enough abstraction via functions. I would like to break the code down into a series of small functions, or methods if i decide to go OOP, but I'm not sure of the best way to do so. If you brilliant Pythonista's could guide me in the write direction I would appreciate it.

Here is the GitHub repo if you would like to fork it repo

'''
script to determine the outcome(s) of any number of dice rolls
By Andrew Smith (2015)
'''

from random import randint

def dice_roller(dice, rolls, sides):
    '''
    Add doc string here.
    '''
    show_di = '|   |'
    show_di_1 = '| * |'
    show_di_2 = '|* *|'
    show_di_3 = '|***|'
    show_di_t = ' ___ '
    show_di_b = ' --- '
    roll_counter = 0
    di_counter = 0

    while roll_counter  ")
        if roll_choice.isalpha(): # if input starts with a letter
            if roll_choice == 'a':
                dice_roller(3, 1, 6)
            elif roll_choice == 'b':
                dice_roller(2, 1, 6)
            elif roll_choice == 'c':
                dice_roller(1, 1, 6)
            elif roll_choice == 'menu':
                print MENU
            elif roll_choice == 'quit':
                again = 1
            else:
                print "Invalid choice.\n"

        elif roll_choice[0].isdigit(): # if input starts with a number
            choices = roll_choice.split()
            size = len(choices)
            if size == 3:
                one = int(choices[0])
                two = int(choices[1])
                three = int(choices[2])
                dice_roller(one, two, three)
            else:
                print "Invalid Choice.\n"
        else:
            print "Invalid choice.\n"

if __name__ == '__main__':
    main()

Solution

Use unpacking

choices = roll_choice.split()
        size = len(choices)
        if size == 3:
            one = int(choices[0])
            two = int(choices[1])
            three = int(choices[2])
            dice_roller(one, two, three)
        else:
            print "Invalid Choice.\n"


Becomes:

dice_roller( * map(int, roll_choice.split()) )


Adding error handling it is:

try:
     dice_roller( * map(int, roll_choice.split()) )
 except Exception as e:
     print("Invalid choice:" + e)


The * operator just goes from a list to free floating arguments and is called the unpacking operator. You use it to pass a list as argument to a function that takes separate arguments.

Naming in full

You should write the names of your variables in full to make them more understandable at first glance. show may be omitted. For example:

  • show_di_1dice_1



  • show_di_bdice_bottom



A dictionary

A dictionary will allow you to avoid the first if elif chain easily. How is left as an exercise for the reader.

Code Snippets

choices = roll_choice.split()
        size = len(choices)
        if size == 3:
            one = int(choices[0])
            two = int(choices[1])
            three = int(choices[2])
            dice_roller(one, two, three)
        else:
            print "Invalid Choice.\n"
dice_roller( * map(int, roll_choice.split()) )
try:
     dice_roller( * map(int, roll_choice.split()) )
 except Exception as e:
     print("Invalid choice:" + e)

Context

StackExchange Code Review Q#111291, answer score: 4

Revisions (0)

No revisions yet.