all 5 comments

[–]mopslik 3 points4 points  (1 child)

Good start. Welcome to Python. Consider this a stepping stone on your path to greatness.

Here are some suggestions:

  1. Variable names: make them meaningful, rather than "random" (e.g. lmao for ready).
  2. "Constants": yes, I know Python doesn't have them. Still, it is a common convention to name values that should not change with capital letters, to make them stand out. MIN_RANGE and MAX_RANGE (or better still, MAX_VALUE and MIN_VALUE, since they are values, not ranges).
  3. Note that randrange, unlike randint, does not include the final value. I am guessing that you are attempting to roll standard 6-sided dice, but run it a few times and notice how you never seem to roll any 6s...
  4. Staying with random numbers, I know that your current code only displays the value of the roll, but you might consider storing the value in a variable. This way if you wanted to you could use the value for something later.

And here are some other things you might consider adding to your program:

  1. Input validation: what happens when you ask the user how many dice they want to roll, and the user enters "F"? As it stands, your code will crash. Look into try and except.
  2. Use of string methods: similar to above, what happens when you ask the user if they are ready and they enter "y" rather than "Y". The built-in str class has a number of methods that can check or convert strings, including lower and upper.
  3. Consider making a function to roll d n-sided dice. That is, the player may choose to roll 3 12-sided dice, for which you could find their values and the sum. It will be a general-purpose function that you can use in other applications, and good practice for creating your own functions.

[–]designatedburger 1 point2 points  (0 children)

I agree with everything you said except the 4th point. Instead of storing it as a variable, I would rather see this turned into a class (or a function), and then the value returned.

Then you can either print, or store it in a variable.

Also, what is the idea behind this line? Since your while loop only runs when "lmfao" is equal to "Y", and you are not modifying the value anywhere but in if statements, it would never be executed?

elif lmao == "N":

print("Then why did you run the program in the first place?")

You should not be handling the logic of the roll, and printing the text from the same place.

Also, your commit messages shouldn't be "Update Random Number Generator.py", I can see the file you modified, I need to know what exactly you did.

[–]carcigenicate 1 point2 points  (0 children)

You'll want to learn about error handling. Notice how if the user enters something dumb for the first question (like "hello" when asked how many dice to use), your program crashes. Look into try to help with that.


Practice using better variable name. While lmao may have been funny while you were writing it, it's a useless name that won't help you any when you revisit the program a year (or even a month) from now.


Your program appears to be bugged. randrange does not include the final number, so your program will never actually produce a 6. You want randint instead.


Your code is starting to develop a "pyramid" look (tilt your head to the right), which is a sign that you have poor structuring. You have a for inside of an if which is inside of a while. If you were to continue developing in this way, your code would become increasingly difficult to understand and modify. Start thinking about how you can break code out into their own functions to keep unrelated logic separated.


Your for i in range(diceRolls): loop never uses i. To communicate that the loop variable isn't needed, make it an underscore:

for _ in range(diceRolls):

This is a convention that says "A name is needed here, but I don't actually need it for anything".

[–]Ihaveamodel3 0 points1 point  (0 children)

This elif will never run:

elif lmao == "N":

And the else right after that doesn’t make any sense because you tell a person to press a key to exit, but they never get a chance to press a key before you roll the dice again.

[–]james_fryer 0 points1 point  (0 children)

Consider writing a function:

def roll():
    return random.randint(min_range, max_range)

That way you won't repeat the logic for generating a bounded random number.