12
\$\begingroup\$

I'm teaching myself code using Zed Shaw's Learn Python The Hard Way, and I got bored during one of the memorization lessons so I thought I would make a random D20 number generator for when I play RPGS.

How can I make this code better? Is there anything stupid I'm doing?

import random

name = raw_input('Please Type in your name > ')

print "\nHello %s & welcome to the Random D20 Number Generator by Ray Weiss.\n" % (name)

first_number = random.randint(1, 20)

print first_number

prompt = ("""
Do you need another number? Please type yes or no.
""")

answer = raw_input(prompt)

while answer == "yes":
    print random.randint(1, 20)
    answer = raw_input(prompt)

if answer == "no":
print "\nThank you %s for using the D20 RNG by Ray Weiss! Goodbye!\n" % (name)

Eventually I would love to add functionality to have it ask you what kind and how many dice you want to role, but for now a review of what I've done so far would really help.

\$\endgroup\$
8
\$\begingroup\$

Here's my take:

from random import randint
name = raw_input('Please Type in your name > ')

print """
Hello {} & welcome to the Random Number Generator by Ray Weiss.
""".format(name)
upper = int(raw_input('Enter the upper limit > '))
n = int(raw_input("How many D{} you'd like to roll? ".format(upper)))

for _ in xrange(n):
    print randint(1, upper)
print """
Thank you {} for using the D{} RNG by Ray Weiss! Goodbye!
""".format(name, upper)

Changes as compared to your version:

  • directly import randint because it's the only function you use in random;
  • use the new string formatting method (str.format);
  • take the upper bound from user instead of hard-coding 20;
  • take the number of rolls from user instead of repeatedly asking if that's enough;
  • use a loop to make the repetition actually work. The self-repeating code asking the user if we should continue is now gone.
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you! This is cool, im gonna play around with it so that it continues to ask you if you want to roll more dice, some new stuff here i havent seen before. mind me asking what the _ in xrange(n) does? I can kind of discern the rest. \$\endgroup\$ – lerugray Sep 20 '12 at 21:41
  • 1
    \$\begingroup\$ @lerugray Here's the doc on xrange. I just use it to run the loop body n times. \$\endgroup\$ – Lev Levitsky Sep 20 '12 at 21:44
  • 1
    \$\begingroup\$ @lerugray, _ in haskell means an empty name (you use it when you must supply a variable which is useless - like in this loop). I suppose it has the same meaning in python. \$\endgroup\$ – Aleksandar Sep 21 '12 at 17:42
  • \$\begingroup\$ @Aleksandar Technically, it's a fully legit name, so it can be used inside the loop, but you got the idea right. I didn't know it came from Haskell (and don't know Haskell), but it makes a lot of sense. \$\endgroup\$ – Lev Levitsky Sep 21 '12 at 20:12
3
\$\begingroup\$

I don't have much to say on the style side, which is good. I think my only real comment would be that I personally find using newline characters simpler than triple-quotes for multiline strings, especially when you just want to make sure of the spacing between lines.

I like that you're using randint for the rolls, instead of randrange or some other structure: it's inclusive for start and stop, and that exactly matches the real-world function that you're recreating here, so there's no need to fudge the parameters or the results with +1.

Design-wise, I would split the front-end stuff, which takes input from the user and gives back information, from the actual dice-rolling. That would let you reuse the dice roller for other purposes (off the top of my head, a random treasure generator), extend the interface logic with additional kinds of functionality, or rework your logic without ripping apart the whole structure.

And as long as you're doing that, think bigger -- 'I need to roll a d20' is just a single case of 'I need to roll some dice', and that problem's not much harder to solve. So here's how I would approach it:

def play():
    """
    This function is just the user interface.
    It handles the input and output, and does not return anything.
    """

    name = raw_input('Please Type in your name > ')

    print "\nHello {}, & welcome to the Random D20 Number Generator by Ray Weiss.\n".format(name)
    print "Please type your rolls as 'NdX' (N=number of dice, X=size of dice), or 'Q' to quit.\n"

    while True:
        dice = raw_input("What dice do you want to roll? ").lower()
        if dice == 'q':
            break
        else:
            try:
                number, size = dice.split('d')
                results = roll_dice(int(number), int(size))
            except ValueError:
                # This will catch errors from dice.split() or roll_dice(),
                # but either case means there's a problem with the user's input.
                print "I don't understand that roll.\n"
            else:
                print 'You rolled {!s}: {!s}\n'.format(sum(results), results)

    print "\nThank you {} for using the D20 RNG by Ray Weiss! Goodbye!\n".format(name)


def roll_dice(number, size):
    """
    number: any int; < 1 is allowed, but returns an empty list.
    size: any int > 1

    Returns: a list with `number` elements, of dice rolls from 1 to `size`
    """
    from random import randint

    return [randint(1, size) for n in range(number)]

One functionality you would probably want to add would be modifying roll_dice() to accept a modifier (+ or - some amount). And if you really want to get fancy, you can start checking the results to highlight 1s or 20s, or other roll results that have special values in your game.

| improve this answer | |
\$\endgroup\$

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.