Generate set of random numbers and remove lowest

Question

I'm learning some Python with my kids, and we're making a program to generate D & D characters from user input.

One portion of the code is to generate stats, which are a set of six numbers where each number is the sum of three random values from 1-6. We're total softies, though, so for each value, we're generating four values from 1-6 and tossing out the lowest. We're also doing this seven times, and throwing out the lowest of the seven, to end up with a list of six values.

import random

stats = []

for i in range(7):
    rolls = []

    for j in range(4):
        roll = random.randint(1,6)
        rolls.append(roll)

    stat = sum(rolls) - min(rolls)
    stats.append(stat)

stats.remove(min(stats))

print stats

It works, but I feel like there might be more elegant solutions/more Pythonic approaches. Any suggestions?

Answer 1

This seems like a nice thing to do with your kids!

Python has these things called comprehensions. For example a list comprehension for the first 5 multiples of 2 could be either:

[i * 2 for i in range(1, 6)]
[i for i in range(11) if i % 2 == 0]

The latter one is a bit harder to understand, and you don't need to know it. So I'll give you an example of what the first one would be if it were not a comprehension.

list_ = []
for i in range(1, 6):
    list_.append(2 * i)

You could change rolls to be a list comprehension, and remove some varibles:

stats = []
for _ in range(7):
    rolls = [random.randint(1, 6) for _ in range(4)]
    stats.append(sum(rolls) - min(rolls))
stats.remove(min(stats))
print stats

---

There's a way you could do this all in two comprehensions. But you are not allowed to define variables in comprehensions. So you would need to use sorted and a slice to get the rolls. I personally think this is harder to understand, and could be a challenge to teach your kids.

roll = sum(sorted(random.randint(1, 6) for _ in range(4))[1:])

However if you think it's nice, you could use it:

stats = [
    sum(sorted(random.randint(1, 6) for _ in range(4))[1:])
    for _ in range(7)
]
stats.remove(min(stats))
print stats

It's a lot shorter than your original, mostly due to not defining variables like roll. But could be harder to understand.

Answer 2

One thing your script is missing, is a function. Functions are extremely useful, and adding a simple one here will be a vital lesson that'll clean up your code. Consider the inner loop you have. It could be changed into a function that returns a list of 4 random rolls, so instead of nesting you call the function, like this:

def roll_dice(number_of_dice):
    rolls = []

    for _ in range(number_of_dice):
        roll = random.randint(1,6)
        rolls.append(roll)

    return rolls

for _ in range(7):
    rolls = roll_dice(4)
    stat = sum(rolls) - min(rolls)
    stats.append(stat)

Now your code is neater, not to mention easier to read and reusable. If you wanted to roll 5 or 6 dice you still can with the same code. Also you could easily add another parameter for number_of_sides so you could roll anything instead of just 6 sided die.

Note I also replaced i and j with _ as it's Pythonic to use an underscore to denote a throwaway value. In your case you only need range for the purpose of looping, so you can throw away the values you're getting.

This may be advanced for your kids, but I'll suggest it anyway as it's very useful. Python has list comprehensions, which are basically for loops condensed into expressions that build a list. For roll_dice you could condense your whole loop like this:

    rolls = [random.randint(1, 6) for _ in range(number_of_dice)]

This cuts down on a lot of lines and to me reads practically like a sentence.

    rolls equals random integer 1-6 for each value in range 1-number_of_dice

You could make a very short and efficent function this way:

def roll_dice(number_of_dice):
    return [random.randint(1, 6) for _ in range(number_of_dice)]

Brevity isn't always the best way to go, but when it reads clearly (as this does to me) it's very useful.

Answer 3

Python 2/3

This is a comment I usually try not to make but as you have mentionned that you and your kids were currently learning, it's probably a good idea to start with the good habits and use Python 3. In your case, it doesn't change much, you just need to add parenthesis when calling print.

Magic numbers

You have a few numbers here and there with no special explanation. Sometimes, just giving them a names makes things easier to understand.

You'd have something like :

NB_ROLL = 7
NB_DICE = 4
NB_FACE = 6

Various

roll is used in a single place, in rolls.append(roll). Maybe you could get rid of the temporary variable.

Then you'd get:

rolls = []
for j in range(NB_DICE):
    rolls.append(random.randint(1,NB_FACE))

which as pointed out in Joe Wallis' answer can be written with a list comprehension:

rolls = [random.randint(1,NB_FACE) for j in range(NB_DICE)]

Code organisation

This is not very relevant here given the size of your code sample but it might be a good idea for learning sake to try to organise the code in small functions.

Also, the code actually doing things (by opposition to just defining things) is usually behind an if __name__ == "__main__": guard to be able to reuse your code more easily.

Probably not useful here but good to keep in mind for your future Python projects :-)

Other data structure

If you want to perform something n times and keep the m best results with bigger values, you might need another algorithm or even better, a more appropriated data structure.

Answer 4

I would suggest attempting to build good coding into your routines from the start of. Therefore I'm going to explain why I consider the following to be be a better solution:

import random

def generate_stats(stats_length, number_of_rolls, roll_min, roll_max):
    """Return a list of stats (of length stats_length) where stat are a 
    sum of number_of_rolls in the range roll_min to roll_max.
    """
    return [sum(random.randint(roll_min, roll_max)
                for _ in xrange(number_of_rolls))
            for _ in xrange(stats_length)]


def main():
    stats = generate_stats(6, 3, 1, 6)
    print("Stats are: {}".format(stats))


if __name__ == '__main__':
    main()

• Use functions for portions of code – You indicate that you are going to do more here, so start using functions already now to simplify later alterations and extensions. I've introduced two functions, one to actually generate the statistics, and one for the main entry point.

Introducing functions also allows for removal of magic numbers like the 4 or 6 or 7. They could also have been handled as constants at top of your code. * Reduce code at top level – All of your code are at the top level, which makes it hard to reuse the code, and when it gets bigger it increases the difficulty of understanding your code. I've introduced the if __name__ == '__main__': line, which calls the main() function. This is a common sight, and would allow for easy understanding of what your code does.

This also allows for another script/program to import your module/script, and use the generate_stats() function directly as part of something larger.

• Document your code – Using docstrings, like the """...""", is the common method to describe functions, and will only be more and more helpful when your code grows bigger. Ideally on one line, but more important that it describes what is happening in the function. This allows for later coding to quickly glance at the docstring, and understand what is happening
• Use list comprehensions – Get used to reading and understanding list comprehension, for they are used a lot. In my version I've skipped the part of removing the lower value, but as indicated in answer by Joe Wallis this can be achieved by sorting and slicing.
• Switch to newer style on print statements – Even though you are using Python 2.7, it is recommended to use the newer style of print functions. See Format string syntax.