3
\$\begingroup\$

I heard that when you toss a die often enough, you could get the value "one" a million times in a row.

I am a beginner in Python 3 and I tried to write a little program that generates 1 or 2 randomly and counts how often the value "one" has occured successively and how big this count could get.

You can also print out the program run or write it to a file to understand the result.

I would like to know if there are possibilities to improve the code quality by using tricks and mechanisms that Python offers to me or if I can reduce something / make something better in general.

from random import randint
import os.path

iterations = int(input("number of iterations: "))
file_with_details = input("create a log file? (y/n): ")
details = input("printing out details about the program run? (y/n): ")
log_file = "is not existing"

# generate folder and log file for program run information
if file_with_details == "y":
    if not os.path.isdir("./logs"):
        os.mkdir("./logs")
    file_number = 1
    file_name = "./logs/log" + str(file_number) + ".txt"
    while os.path.isfile(file_name):
        file_number += 1
        file_name = "./logs/log" + str(file_number) + ".txt"
    log_file = open(file_name, "w")

# start of random experiment
biggest_succession = 0
actual_succession = 0
random_number_before = 0

for i in range(iterations):
    random_number = randint(1, 2)
    if details == "y":
        print("generated number:", random_number)
    if file_with_details == "y":
        log_file.write("generated number:" + str(random_number) + "\n")

    if random_number == 1 and random_number_before == 1:
        actual_succession += 1
        if details == "y":
            print("actual succession:", actual_succession)
        if file_with_details == "y":
            log_file.write("actual_succession:" + str(actual_succession) + "\n")
    else:
        if actual_succession > biggest_succession:
            biggest_succession = actual_succession
            if details == "y":
                print("biggest succession has changed:", biggest_succession)
            if file_with_details == "y":
                log_file.write("biggest succession has changed:" + str(biggest_succession) + "\n")
        actual_succession = 0
    random_number_before = random_number

print("the number one was generated", biggest_succession, "times behind each other")
if file_with_details == "y":
    log_file.write("the number one was generated " + str(biggest_succession + 1) + " times behind each other")
    log_file.close()
\$\endgroup\$
3
\$\begingroup\$
  1. If your program is going to be run via cli only, then I'd suggest that you look into argparse library.
  2. Split the code snippet into different functions. Though your current use-case is limited, but suppose you later want to check successive outcomes in a 6 face die? A 10 face die? A 20 face die? I'd suggest asking for number of faces as an argparse param, and default it to 2.
  3. Use the logging library's RotatingFileHandler to control logging to the files.
  4. In the logic, you do not need to store the value of random_number_before. Just checking against a constant value of 1 would suffice in your case:

    if random_number == 1:
        actual_succession += 1
    else:
        if actual_succession > longest_succession:
            longest_succession = actual_succession
        actual_succession = 0
    

    as the logic is executed for each iteration of the counter, everytime you encounter a number other than 1, your succession counter would be reset to 0.

| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

Have you checked that the output being printed to console is the same as the output being written to file?

You want to know the # of successive 1's, so you should update the value of the variable to reflect that rather than updating it in your print statement, which may lead to cases where you forget the variable you are accessing is not actually what you want.

You also don't need to update random_number_before when both random_number and random_number_before are 1, so you can move it into your else block. However, as @hjpotter2 pointed out, you don't even need the variable.

Your print and log statements are essentially the same, 1. check if variable is true and if so print/log some strings. You could write a general function to handle this:

def print_or_log(strs, log=False):
    if log and file_with_details=="y":
        log_file.write(" ".join(strs + ["\n"]))
    elif details == "y" and not log:
        print(" ".join(strs))

Then just pass in lists of your output

print_or_log(["actual succession", str(actual_succession)],log=True)

and this would reduce the lines of your print/log statements by half.

Alternatively, if you don't care about logging each step, you could observe that you are trying to separate values from each other. You could then take advantage of Python's string operations and list comprehensions to ditch the for loop and really simplify your algorithm.

Generate your random numbers and combine them into a string. Then split on "2" to create lists of consecutive ones and just find the max length to find the longest consecutive sequence.

random_numbers = "".join(str(random.choice([1,2])) for _ in range(iterations))
longest_succession = max(len(x) for x in random_numbers.split("2"))

And you are done. If you want multiple values, not just 1 and 2, you could do the same thing with numpy instead of random. The disadvantage is that in this case you are limited by your memory. The advantage is that you could implement your entire algorithm in 2 clear lines of code with no counters.

| improve this answer | |
\$\endgroup\$
  • 2
    \$\begingroup\$ Storing all generated numbers in memory is a bad design, considering that there may be 10^9 iterations. \$\endgroup\$ – hjpotter92 Oct 6 '17 at 11:30
  • \$\begingroup\$ @hjpotter92 true, I did mention the space problem. However, he asked for a more pythonic solution and in the majority of cases where the user doesn't hit the the memory ceiling, the list comprehension is a much simpler and elegant solution. It really depends on the scope of his user input. \$\endgroup\$ – mochi Oct 7 '17 at 2:39

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.