3
\$\begingroup\$

This is one of my mini project of dice rolling and I want to improve it further with more advanced Python. Maybe you have any ideas to improve the code itself?

This code will ask the minimum and maximum number for the dice from the user and the number of times it will roll them, giving random numbers.

import random #Size choosing for dice while True: #Choosing the minimum number Min = input("Please input the minimum number of your dice: ") try: Min = int(Min) except ValueError: print("Invalid input") continue #Choosing the maximum number Max = input("Please input the maximum number of your dice: ") try: Max = int(Max) except ValueError: print("Invalid input") continue #Check if the minimum and maximum is valid if Min > Max: print("Minimum is bigger than Maximum.\n Please reinput data...") continue elif Min == Max: print("Minimum is equaled to Maximum,\n Please reinput data...") continue print(f"Your dice is from {Min} to {Max}.") break #Random number generater while True: Rollcount = input("How many times do you want to roll the dice: ") try: Rollcount = int(Rollcount) break except ValueError: print("Invalid input") continue for i in range(0,Rollcount): roll = random.randint(Min,Max) print(roll) 
\$\endgroup\$
0

2 Answers 2

1
\$\begingroup\$

The easiest way to improve this code is to write a generic function that asks for user input. It should be able to handle types and a validator function and keep on asking until a valid input is given. Something like this:

def ask_user(message, type_=str, valid=lambda x: True, invalid_message="Invalid"): while True: try: user_input = type_(input(message)) except (ValueError, TypeError): print("Invalid input") continue if valid(user_input): return user_input else: print(invalid_message) 

With which your code becomes a lot easier:

import random min_ = ask_user("Please input the minimum number of your dice: ", int) max_ = ask_user("Please input the maximum number of your dice: ", int, lambda x: x > min_, "Maximum must be larger than minimum.") n = ask_user("How many times do you want to roll the dice: ", int) rolls = [random.randint(min_, max_) for _ in range(n)] for roll in rolls: print(roll) 

Note that I followed Python's official style-guide, PEP8, which recommends using lower_case for functions and variables and PascalCase only for classes. I avoided shadowing the built-in functions min, max and type by adding a trailing _, which is what I also used for the unused loop variable. I also used a list comprehension, so you have all the rolls already in a list in case you need them later.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ i am a total beginner and does not understand some part of the code even though it works completely fine. i just wanna ask why is the type_ equals to str instead of int. should't it be int as its a number. The 2nd question is what is the messgae in the function? \$\endgroup\$ Commented May 18, 2020 at 6:39
  • \$\begingroup\$ @ShreddedPumpkin That was just the default value I chose for the type. When calling the function I always passed int. I could also have used int as the default value, but then the function should probably be called ask_number. However, I chose str so the function is more generally useful, and not only for this specific usecase. In the general case the safest choice for a type is not making a choice at all and just do what input does, namely return a string. \$\endgroup\$ Commented May 18, 2020 at 6:42
  • \$\begingroup\$ @ShreddedPumpkin The message is what is printed when the user input does not pass the validation, i.e. in your case if the second number ist not larger than the first. One could also add another parameter for the message being printed when the type cast does not work, but I did not want to clutter the function with things not needed in your code. \$\endgroup\$ Commented May 18, 2020 at 6:44
2
\$\begingroup\$

I normally would agree with Graipher here, however with your code I think there's a simpler and more readable solution.

  1. The first number doesn't have to be larger than the second.

    This is an artificial limitation that can be fixed by sorting the inputted values.

  2. Not allowing the same value to be selected is an artificial limitation. random.randint includes both bounds, meaning random.randint(n, n) is valid input.

  3. You can bundle the first two try statements together to simplify the code.

    This makes the code DRY as the two loops are logically doing two different things.

Whilst longer it keeps the functionality to change the lower bound if you enter it incorrectly, without having to kill the process.

import random while True: try: min_, max_ = sorted([ int(input("Please enter one of the bounds: ")) int(input("Please enter the other bound: ")) ]) break except ValueError: print("Invalid input") while True: try: amount = int(input("How many times do you want to roll the dice: ")) break except ValueError: print("Invalid input") for _ in range(amount): print(random.randint(min_, max_)) 

Note: You can move both the while try loops into a function, and pass a lambda, however I don't think that's as readable.

min_, max_ = while_try(lambda: sorted([ int(input("Please enter one of the bounds: ")) int(input("Please enter the other bound: ")) ])) 
\$\endgroup\$

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.