5
\$\begingroup\$

I am a fairly new programmer, and I stumbled with a beginner practice project, a random dice number generator. The code works, but I would like to know if there is an efficient way compared to mine to write it.

#Random dice 
import random 

#returns a number (side of the dice)
def roll_dice():
  print (random.randint(1, 6)) 

print("""
Welcome to my python random dice program!
To start press enter!
Whenever you are over, type quit.
""")

flag = True
while flag:
   user_prompt = input(">")
   if user_prompt.lower() == "quit":
      flag = False
   else:
     print("Rolling dice...\nYour number is:") 
     roll_dice()
\$\endgroup\$
6
\$\begingroup\$

You can make it look nicer!

Instead of writing comments in front of your functions, do it with docstrings:

def roll_dice():
    """Print a number between 1 and 6 (side of the dice)"""
    print(random.randint(1, 6))

You can observe that I made a couple of changes to this:

  • removed the extra space you had in your print() function
  • added the docstring I mentioned above
  • modified the content of docstring (your function doesn't return anything, it just prints a random number). A beginner programmer might get the wrong idea.
  • used 4-spaces indentation instead of two.
  • 2 new lines in front of your function

Now, let's try to make it even better!

You have two magic numbers, 1 and 6. Since you put the logic inside a function, let's make use of it and define those as arguments to our function:

def roll_dice(min_dice, max_dice):
    """Print a number between min_dice and max_dice (side of the dice)"""
    print(random.randint(min_dice, max_dice))

The above has the advantage of an easier customization of your program. More, you can abstract things even more, and give those arguments a default value:

def roll_dice(min_dice=1, max_dice=6):
    """Print a number between min_dice and max_dice (side of the dice)"""
    print(random.randint(min_dice, max_dice))

More, you can go a step further and make the function actually do something. Let's make it so that it returns a value. Just replace the print() function with return.

Okay, so far so good. I think we've managed to handle this part pretty well. Let's move on.

First, let's apply the changes we did in the first part, to this one too:

print("""
Welcome to my python random dice program!
To start press enter!
Whenever you are over, type quit.
""")

flag = True
while flag:
    user_prompt = input(">")
    if user_prompt.lower() == "quit":
        flag = False
    else:
        print("Rolling dice...\nYour number is:")
        roll_dice()

What I don't like about this, is the fact that you didn't wrap the logic inside a function. Let's do that first:

def play():
    print("""
    Welcome to my python random dice program!
    To start press enter!
    Whenever you are over, type quit.
    """)
    flag = True
    while flag:
        user_prompt = input("> ")
        if user_prompt.lower() == "quit":
            flag = False
        else:
            print("Rolling dice...\nYour number is:")
            roll_dice()

The changes that I'd like to make to this function are the following:

  • remove the flag variable
  • move out the intro message in it

    def play():
        while True:
            user_prompt = input("> ")
            if user_prompt.lower() == "quit":
                return False
            else:
                print("Rolling dice...\nYour number is: {}".format(roll_dice()))
    

Moving next, let's build our main() function:

def main():
    print("Welcome to my python random dice program!\n"
          "To start press ENTER!\n"
          "Whenever you are over, type quit.\n")
    play()

Last but not least, let's call our main function:

if __name__ == "__main__":
    main()

You can see I added an extra line: if __name__ == "__main__". By doing the main check, you can have that code only execute when you want to run the module as a program and not have it execute when someone just wants to import your module and call your functions themselves.


The full code:

import random


def roll_dice(min_dice=1, max_dice=6):
    """Print a number between min_dice and max_dice (side of the dice)"""
    return random.randint(min_dice, max_dice)


def play():
    """Return false if user enters 'quit'. Otherwise, print a random number"""
    while True:
        user_prompt = input("> ")
        if user_prompt.lower() == "quit":
            return False
        else:
            print("Rolling dice...\nYour number is: {}".format(roll_dice()))


def main():
    print("Welcome to my python random dice program!\n"
          "To start press ENTER!\n"
          "Whenever you are over, type quit.\n")
    play()

if __name__ == "__main__":
    main()
\$\endgroup\$
  • 1
    \$\begingroup\$ Since there is no specific tag on the question, it's probably worth noting that this is ok in python 3, while in python 2 raw_input() should be used. \$\endgroup\$ – ChatterOne May 12 '17 at 6:46
  • 2
    \$\begingroup\$ A simple way to fix ChatterOne's problem is to use the try: try: input = raw_input except NameError: pass. But I think it's reasonably safe to believe OP is using Python 3. \$\endgroup\$ – Peilonrayz May 12 '17 at 8:42
  • \$\begingroup\$ Thanks for the response. I've learned a lot (btw I'm using python 3). \$\endgroup\$ – Jfelix May 12 '17 at 13:53

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.