1
\$\begingroup\$

Is there a way to write this code more efficiently so that it is easier to read, maybe using loops and lists?

import random
total = 0
sixes = 0
fives = 0
fours = 0
threes = 0
twos = 0
ones = 0
limit = 50
throws = 0
Total = 0

play = input("Do you want to play, 'Y' or 'N'")
if play.upper() == "Y" :
    play = True
else :
    play = False


while total < limit and play :
    roll = random.randint(1,6)
    throws +=1
    total += roll
    if roll == 6:
        sixes +=1
    elif roll == 5:
        fives +=1
    elif roll == 4:
        fours +=1
    elif roll == 3:
        threes +=1
    elif roll == 2:
        twos +=1
    elif roll == 1:
        ones +=1
    else :
        print(" try again ")

if play :
    print("Total throws : ",throws)
    print("Total : ",total)
    print("Total 6's : ",sixes)
    print("Total 5's : ",fives)
    print("Total 4's : ",fours)
    print("Total 3's : ",threes)
    print("Total 2's : ",twos)
    print("Total 1's : ",ones)
else :
    print("Yr los")
\$\endgroup\$
1
  • \$\begingroup\$ Could you please add some more context to your post, like what your code does, and if there is anything specifically that you would like assistance with? \$\endgroup\$ – SirPython Sep 22 '15 at 20:50
6
\$\begingroup\$

Your program is quite static, and as you probably know that's not good.

First I would recommend changing the entire throws, ones, twos, etc logic to a list.

# Lazily initialise it with 6 zeros
throws = [0] * 6

This will now be where we hold ones, twos, etc but not throws or total.

The way you should use it is by indexing it.

if roll == 6:
    throws[5] += 1
elif roll == 5:
    throws[4] += 1
# etc

As you can probably guess, you can just index it with roll - 1. And that is right! Below is most of your old code in a few lines.

total = 0
throws = [0] * 6
while score < limit and play:
    roll = random.randint(1, 6)
    throws[roll - 1] += 1
    total += roll

Now all there is to look at is your prints. You can use str() to convert a number to a string. And so you can automate most of your prints.

for side, amount = enumerate(throws, 1):
    print('Total ' + str(side) + "'s:", amount)

First enumerate will loop through the list, giving you the index and item. Or in our case side and amount. The above print looks kinda ugly, and so you can change the print to use str.format.

print('Total {}\'s: {}'.format(side, amount))

This replaces the first {} with the value of side, and the second {} with amount. So if side was 1, and amount was 2 it would display Total 1's: 2.

In summery, your code will look something like this:

import random

if input("Do you want to play, 'Y' or 'N'").upper() == "Y" :
    play = True
else :
    play = False

limit = 50

total = 0
throws = [0] * 6
while score < limit and play:
    roll = random.randint(1, 6)
    throws[roll - 1] += 1
    total += roll

if play :
    print("Total throws : ", sum(throws))
    print("Total : ", total)
    for side, amount in enumerate(throws, 1):
        print('Total {}\'s: {}'.format(side, amount))
else:
    print("Yr los")

To further improve your program I would recommend that you put all the 'play' functionality into a function and make your program work with any sized dice.

def play(limit, size):
    total = 0
    throws = [0] * size
    while score < limit:
        roll = random.randint(1, size)
        throws[roll - 1] += 1
        total += roll

    print("Total throws : ", sum(throws))
    print("Total : ", total)
    for side, amount in enumerate(throws, 1):
        print('Total {}\'s: {}'.format(side, amount))

if input("Do you want to play, 'Y' or 'N'").upper() == "Y" :
    play(50, 6)
    play(500, 20) # Limit = 500, dice size = 20
else:
    print("Yr los")
\$\endgroup\$
1
  • \$\begingroup\$ You forgot to index with roll - 1. \$\endgroup\$ – SuperBiasedMan Sep 23 '15 at 8:23
3
\$\begingroup\$

Using idiomatic set-comprehension and Counter makes this so much shorter and clearer:

>>> from collections import Counter
>>> import random
>>> throws = (random.randint(0, 5) + 1 for _ in range(10000))
>>> Counter(throws)
Counter({3: 1696, 5: 1683, 1: 1663, 6: 1658, 2: 1654, 4: 1646})

Further improvement over this would be extracting the number of sides and the number of throws into a constant, writing a user interface, allowing multiple dices to be thrown at once and summing them, printing more human-friendly output, extracting a roll_die function ...

\$\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.