4
\$\begingroup\$

I've just begun my journey into python (1 week now) and I've written couple of lines incorporating everything I've learned so far. It would be greatly appreciated if anyone could guide me in writing a better formatted (appropriate style) version of this.

I've just written a couple of lines on finding two random rolls that stop when they roll the same number. When the same number is rolled, that number is recorded along with how many rolls it took to get there. I've also made the code to find those values and store them in a list, finding the average of roll number(same number) as well as roll count(How many rolls it took). Here is the code:

import random
totalrolls = 0
rollList = []
rollcountlist = []
while totalrolls < 500:
    totalrolls = totalrolls + 1
    rollcount = 0
    roll = random.randint(1,100)
    rolltwo = random.randint(1,100)
    while roll != rolltwo:
        roll = random.randint(1,100)
        rolltwo = random.randint(1,100)
        #print roll
        #print rolltwo
        rollcount = rollcount + 1
    if roll == rolltwo:
        rollList.append(roll)
        rollcountlist.append(rollcount)
        print "Random number that matched was: %s, after %s rolls. " % (roll, rollcount)
if totalrolls == 500:
    print "Finished rolling %s times." % totalrolls

averageroll = sum(rollList) / len(rollList)
averagerollcount = sum(rollcountlist) / len(rollcountlist)

print rollList
print rollcountlist
print averageroll
print averagerollcount
\$\endgroup\$
4
\$\begingroup\$

You have an off-by-one error. If you get lucky and get a matching pair immediately, then you would report "Random number that matched was: …, after 0 rolls." I would say that "after 1 roll(s)" would be the right way to report that. (Alternatively, you could phrase it as "after 0 mismatched rolls".)

Naming could be improved slightly. I suggest:

  • rollcountlistroll_counts, because it's shorter and more readable. Since englishisnotgerman, you should use an underscore, as recommended by PEP 8.
  • rollListroll_values, because "values" makes it clearer what is being stored. Watch your capitalization consistency.
  • averagerollaverage_value and averagerollcountaverage_count, for similar reasons.
  • totalrollsexperiment_count, because totalrolls sounds like it represents a cumulative sum of rollcount, which it isn't. Introducing the term "experiment" makes it clear that you are counting something else.
  • rollroll1 and rolltworoll2, for consistency. Or, as I've done below, make one two-element tuple called rolls.

The if conditions after the while loops are superfluous. The inner loop can only be exited when the roll values match. The outer loop can only be exited after 500 experiments.

Counting loops can be more elegantly expressed. For a fixed number of iterations, such as for the outer loop, use the idiom for i in xrange(n): …. For an unbounded count, such as for the inner loop, use for i in itertools.count().

In Python 2, the / operator performs integer division (rounding down) when dividing an integer by an integer. For a more accurate mean, convert at least one of the operands to a float first.

When interpolating an integer into a format string, use the %d placeholder instead of %s for clarity.

from itertools import count
import random

roll_values = []
roll_counts = []
for experiment_count in xrange(1, 501):
    for roll_count in count(1):
        rolls = (random.randint(1,100), random.randint(1, 100))
        if rolls[0] == rolls[1]:
            break
    roll_values.append(rolls[0])
    roll_counts.append(roll_count)
    print "Random number that matched was: %d, after %d rolls." % (rolls[0], roll_count)

average_value = float(sum(roll_values)) / len(roll_values)
average_count = float(sum(roll_counts)) / len(roll_counts)

print "Finished rolling %d times." % experiment_count    
print roll_values
print roll_counts
print average_value
print average_count
\$\endgroup\$
  • \$\begingroup\$ Wow. Thank you so much for your help! It was exactly what I needed to learn python but also to learn the most efficient way and the smarter way to write python. \$\endgroup\$ – John Lee Nov 3 '17 at 12:36
2
\$\begingroup\$

@200_succes has some very good advise, but I think I can add some more.

Originally this was supposed to be a comment, but it got to long.

  • the % has a few issues, better to use .format(). This will work in both python2 and python3
  • Even better yet, why not use python3 ?

Especially if you're new learning Python, I suggest to learn python3. Support for python2 will be dropped soons since Python2 EOL (End Of Life) is just a few years away.

It seems silly to learn a language that will be dropped soon.

\$\endgroup\$
  • \$\begingroup\$ I just learned about .format() and I'll definitely give that a go. \$\endgroup\$ – John Lee Nov 3 '17 at 12:36
  • \$\begingroup\$ and yes, seems like learning python 3 would be a better idea. \$\endgroup\$ – John Lee Nov 3 '17 at 12:37
  • \$\begingroup\$ @JohnLee Not a problem, happy to help. Your code will work in python3 if you change your print xxx statements to print (xxx) and use .format() \$\endgroup\$ – Ludisposed Nov 3 '17 at 12:40

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.