Die-rolling program

Question

I'm getting into learning python so to learn I made a die-rolling application. What would you do differently? What would you suggest to improve performance/readability? What tips do you have? Suggest pretty much anything.

import random
import time
import re
#installed via pip install inflect (https://pypi.org/project/inflect/) 
import inflect
while True:
 confirmation = input("Would you like to roll some dice?(y/n) > ")
 if len(confirmation) != 1:
 print("Error! You may only input one character")
 elif not re.match(r"^[yn]$", confirmation, flags=re.IGNORECASE): 
 print("Error! Only y and n are valid")
 else :
 break
if confirmation.casefold() == "n" :
 print("No dice have been rolled")
elif confirmation.casefold() == "y":
 while True:
 count = input("How many dice would you like to roll? > ")
 if len(count) == 0 :
 print("Error! Please input something!")
 elif not re.search(r"^[0-9]*$", count) :
 print("Error! You may only input numbers")
 else :
 p = inflect.engine()
 for i in range(1, int(count)+1) :
 result = random.randint(1,6)
 print("The " + p.number_to_words(p.ordinal(i)) + " die rolled a: " + str(result))
 break

Answer 1

Specific suggestions

1. The while True loop at the top is, well, pointless. Why have a section in the program asking whether the user wants to (effectively) run the program?

2. re.match(r"^[yn]$", confirmation, flags=re.IGNORECASE) implies len(confirmation) == 1, so you could consider the length check redundant. The same goes for count.

3. You end up effectively converting the case of confirmation thrice:

   • re.match(r"^[yn]$", confirmation, flags=re.IGNORECASE)
   • confirmation.casefold() == "n"
   • confirmation.casefold() == "y"

   By instead converting confirmation to lowercase before doing any of these you can do a case sensitive match, which is simpler code and faster.

4. r"^[0-9]*$" includes matches for the empty string. What you probably want is r"^[0-9]+$", which looks for at least one number.

5. The last else should just run break; then p = inflect.engine() etc can be moved outside the loop for clarity.

6. This code isn't really reusable. Almost 100% of Python scripts you will ever find in production systems will be reusable. This means they are:

   • Non-interactive. In this case, that means removing the inputs, instead either using argparse or something to get a single parameter containing the number of dies you want to print, or very simply just printing one die number.

   • Importable. Typically there are three parts to this: an entrypoint (in your case this could be a very simple function which prints a single random number in the specified range), a main method which takes care of reading command line parameters and running the entrypoint, and this magic:

      if __name__ == "__main__":
      main()
     

   The last bit runs the main method when the script is called from the command line. The entrypoint can then be imported from any other script and reused trivially.

Tool support suggestions

1. black can automatically format your code to be more idiomatic. It'll do things like adjusting the vertical and horizontal spacing, while keeping the functionality of the code unchanged.

2. flake8 can give you hints to write idiomatic Python. I would start with this configuration:

    [flake8]
    max-complexity = 4
    ignore = W503,E203
   

   This would for example detect that you have a redundant import (time).

• l0b0: A confirmation step is only relevant when the script does something potentially destructive. Printing to screen is not destructive, and consider that the confirmation prompt prints much more than the actual functionality of the script... About learning to use input I can only say that it's unlikely to be useful in practice. It's an unfortunate practice in education to always teach how to have an interactive two-way conversation when really you just want the code to do a thing with minimal fuss.
• Kadragon: 1) This I find is a very common disagreement among a lot of programmers. Some preferring a confirmation step for safety. Others not feeling any need. But the while loop was there to learn how to keep asking for y/n if you were to input let's say "a" 2) I do see how that's redundant, thank you 3) I do see how it is more efficient to do casefold earlier on and only once 4) I can see how "+" is more clear, but the "*" is still working as intended, only returning when there is at least one number
• Kadragon: 5)) It is not run every iteration of the loop. It is not in the for loop, and the loop only gets to that elif once in the program. But if I were to do something else it would make it run every time possibly. 6) Can you go into this more? I don't understand fully. I will also look into these tools
• l0b0: "This I find is a very common disagreement among a lot of programmers." In about 10 years of using Python I don't think I've ever used input. I've only ever seen it in toy code.
• l0b0: 'the "*" is still working as intended, only returning when there is at least one number.' No, * means zero or more of the preceding expression. The only reason it works is that you also check the length separately, which is unnecessary if you use + instead.

Answer 2

If these die rolls are going to be used for anything at all important, it's better to use the secrets module rather than random.

import secrets
...
rng = secrets.SystemRandom()
...
 result = rng.randint(1, 6)

The random module provides numbers which are statistically random, but not cryptographically secure; in principle (and if there's enough money involved, also in practice), someone could work out what the next number is likely to be.

• Kadragon: I will keep that in mind. This isn't being used for anthing important, but the secrets module will be someting worth keeping in mind.

Answer 3

A few things in your code seem redundant (Most of these have been remarked well by previous answers) - Although perhaps I am missing some specific reasons for your choices and its also good to remember there are tonnes of answers for programming questions - This is my take on your functionality aim:

import random
import inflect
p = inflect.engine()
while True:
 confirmation = input("Would you like to roll some dice?(y/n) > ")
 if confirmation == "y":
 while True:
 count = input("How many dice would you like to roll? > ")
 if len(count) == 0:
 print("Error! Please input something!")
 elif not count.isnumeric():
 print("Error! You may only input numbers")
 else:
 [print("The " + p.number_to_words(p.ordinal(i)) + " die rolled a: " 
 + str(random.randint(1, 6))) for i in range(1, int(count))]
 break
 elif confirmation == "n":
 print("No dice have been rolled")
 else:
 print("Error! Only y and n are valid") if len(confirmation) > 1 else print("Error! You may only input one character")
```