Dice-rolling program

I am sort of new to programming. I would like to understand the code areas that are garbage and could be better written keeping the exact (if possible) behavior the code has now.

```
from random import randint
import time
from collections import OrderedDict

cast_again = 'r'
statistics = OrderedDict()
casts = 0
menu_options = """
> Options:
> R - to cast again;
> S - view stats;
> E - exit the programme.
> """

def rollDice(dice):
input('> Press Enter to cast dice!')
time.sleep(1)
cast_result = []
for a_cast in range(0,dice):
cast_result.append(randint(1,6))
return cast_result

def atLeastALetterInput(text_to_display):
cast_again = 'x'
try:
cast_again = input(text_to_display)[0].lower()
return cast_again
except IndexError:
print('> Please make sure input is valid before pressing enter...')

while 1:
try:
num_dice = int(input('> Enter the number of dice you would like to cast! (max 10):'))
if num_dice not in range(0,11):
print('> Please enter a value between 1 and 10! ')
continue
break
except ValueError:
print('> Non-numerical characters detected. Try again!')

while 1:
if cast_again not in ('r', 's', 'e'):
print('> Ups! Available options R(new cast), S(statistics) and E(exit).')
cast_again = atLeastALetterInput(menu_options)
continue
if cast_again == 'r':
next_cast = casts + 1
print('> Here comes cast number {}'.format(next_cast))
casts +=1
cast_results_to_stats = rollDice(num_dice)
statistics[casts] = cast_results_to_stats
print('> Here is your result: {}'.format(cast_results_to_stats))
cast_again = atLeastALetterInput(menu_options)
elif cast_again == 's':
for key in statistics:
print('> *** Cast {} resulted in {}'.format(key, statistics[key]))
cast_again = atLeastALetterInput(menu_options)
elif cast

About rollDice

The name of the function does not follow PEP 8, the Python style guide.

The name of the parameter is not that great.

You don't need 0 as a first parameter of range as it is the default behavior.

The value a_cast is not used. Usually, the name for throw-away values in Python is _.

The loop could be written in a list compregension.

After taking into account these details, your code looks like :

def roll_dice(nb_dice):
    input('> Press Enter to cast dice!')
    time.sleep(1)
    return [randint(1,6) for _ in range(nb_dice)]

About atLeastALetterInput

Here again, the name does not seem correct.

The variable is not required.

After rewriting this, code looks like :

def get_letter_from_user(text_to_display):
    try:
        return input(text_to_display)[0].lower()
    except IndexError:
        print('> Please make sure input is valid before pressing enter...')

About your first while loop

It should probably be put in a function with a signature like def get_nb_from_user(min_val, max_val):.

You could remove the need for a continue by re-organising the conditions :

num_dice = int(input('> Enter the number of dice you would like to cast! (max 10):'))
    if num_dice in range(0,11):
        break
    else:
        print('> Please enter a value between 1 and 10! ')

Checking if an integer is in an interval by checking if it is in the result from range is not efficient at all : if you were to check if a number is between 1 and 100000000, it would take a very long time even though this should be pretty simple.

Your code becomes :

def get_nb_from_user(min_val, max_val):
    while True:
        try:
            nb = int(input('> Enter the number of dice you would like to cast! (max %s):' % max_val))
            if min_val  Please enter a value between %s and %s! ' % (min_val, max_val))
        except ValueError:
            print('> Non-numerical characters detected. Try again!')

num_dice = get_nb_from_user(1, 10)

About your second while loop

To keep things simple, you should try to call get_letter_from_user from a single place.

This is quite easy to do :

while True:
    cast_again = get_letter_from_user(menu_options)
    if cast_again == 'r':
        next_cast = casts + 1
        print('> Here comes cast number {}'.format(next_cast))
        casts +=1
        cast_results_to_stats = roll_dice(num_dice)
        statistics[casts] = cast_results_to_stats
        print('> Here is your result: {}'.format(cast_results_to_stats))
    elif cast_again == 's':
        for key in statistics:
            print('> *** Cast {} resulted in {}'.format(key, statistics[key]))
    elif cast_again == 'e':
        print('> The game finished after {} casts.'.format(casts))
        print('> Good bye! ')
        input()
        break
    else:
        print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

Then again, as explained in a different answer, there is no need for an ordered dictionnary as a list would do the trick.

Then your code begins (variable definition included) :

num_dice = get_nb_from_user(1, 10)
statistics = []

while True:
    cast_again = get_letter_from_user(menu_options)
    if cast_again == 'r':
        print('> Here comes cast number {}'.format(len(statistics)+1))
        cast_results_to_stats = roll_dice(num_dice)
        statistics.append(cast_results_to_stats)
        print('> Here is your result: {}'.format(cast_results_to_stats))
    elif cast_again == 's':
        for i, cast in enumerate(statistics):
            print('> *** Cast {} resulted in {}'.format(i, cast))
    elif cast_again == 'e':
        print('> The game finished after {} casts.'.format(len(statistics)))
        print('> Good bye! ')
        input()
        break
    else:
        print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

Now, in Python, it is usually a good thing to move the part of your code actually doing stuff behind a main guard. Thus, your code begins :

def main():
    """Main function"""
    num_dice = get_nb_from_user(1, 10)
    statistics = []

    while True:
        cast_again = get_letter_from_user(menu_options)
        if cast_again == 'r':
            print('> Here comes cast number {}'.format(len(statistics)+1))
            cast_results_to_stats = roll_dice(num_dice)
            statistics.append(cast_results_to_stats)
            print('> Here is your result: {}'.format(cast_results_to_stats))
        elif cast_again == 's':
            for i, cast in enumerate(statistics):
                print('> *** Cast {} resulted in {}'.format(i, cast))
        elif cast_again == 'e':
            print('> The game finished after {} casts.'.format(len(statistics)))
            print('> Good bye! ')
            input()
            break
        else:
            print('> Ups! Available options R(new cast), S(statistics) and E(exit).')

if __name__ == "__main__":
    main()

T

StackExchange Code Review Q#59671, answer score: 7