2
\$\begingroup\$

I just finished building a simple random Tic Tac Toe generator. The program will display 3 rows of a random numbers 0-1 and will print a message when 3 of the same numbers align. The program works perfectly, to my knowledge.

I am posting this code here in hopes that I can gain some advice/tips in regards to optimization and/or simplification.

import random

row1 = [round(random.uniform(0, 1)), round(random.uniform(0, 1)), round(random.uniform(0, 1))]
row2 = [round(random.uniform(0, 1)), round(random.uniform(0, 1)), round(random.uniform(0, 1))]
row3 = [round(random.uniform(0, 1)), round(random.uniform(0, 1)), round(random.uniform(0, 1))]

print(row1)
print(row2)
print(row3)

# If statements for all 3 rows
if (row1[0] == row1[1]) and [row1[1] == row1[2]] and (row1[2] == row1[0]):
    print("All " + str(row1[0]) + "'s in top row!")
if (row2[0] == row2[1]) and [row2[1] == row2[2]] and (row2[2] == row2[0]):
    print("All " + str(row2[0]) + "'s in middle row!")
if (row3[0] == row3[1]) and [row3[1] == row3[2]] and (row3[2] == row3[0]):
    print("All " + str(row3[0]) + "'s in bottom row!")

# If statements for all 3 columns
if (row1[0] == row2[0]) and [row2[0] == row3[0]] and (row3[0] == row1[0]):
    print("All " + str(row1[0]) + "'s in left column!")
if (row1[1] == row2[1]) and [row2[1] == row3[1]] and (row3[1] == row1[1]):
    print("All " + str(row1[1]) + "'s in middle column!")
if (row1[2] == row2[2]) and [row2[2] == row3[2]] and (row3[2] == row1[2]):
    print("All " + str(row1[2]) + "'s in right column!")

# If statements for diagonals
if (row1[0] == row2[1]) and (row2[1] == row3[2]) and (row3[2] == row1[0]):
    print("All " + str(row1[0]) + "'s in diagonal!")
if (row1[2] == row2[1]) and (row2[1] == row3[0]) and (row3[0] == row1[2]):
    print("All " + str(row1[2]) + "'s in diagonal!")
\$\endgroup\$
2
\$\begingroup\$

Some suggestions:

  • There’s a lot of repetition in the code for generating each of the rows. Repetition is generally a bit of a code smell; you could improve this by using a list comprehension for each row, for example:

    row1 = [round(random.uniform(0, 1)) for _ in range(3)]
    row2 = [round(random.uniform(0, 1)) for _ in range(3)]
    row3 = [round(random.uniform(0, 1)) for _ in range(3)]
    

    By nesting the list comprehension and using tuple unpacking, we can reduce repetition further:

    row1, row2, row3 = [
        [round(random.uniform(0, 1)) for _ in range(3)]
        for _ in range(3)
    ]
    

  • Your code uses 1.0 and 0.0 for the two values, whereas convention is usually to use O and X. If you use random.choice instead of random.uniform, you can get this instead:

    row1, row2, row3 = [
        [random.choice(['O', 'X']) for _ in range(3)]
        for _ in range(3)
    ]
    

    It’s good that the rest of your code copes with this gracefully – it doesn’t actually care what values you’re putting in the cells.

    Once you’ve done that, you can tidy up the printing of the rows so you don't see the square brackets/commas from Python’s list syntax.

  • There are lots of if statements with multiple conditions and repetition, which is a bit messy. It would be cleaner if you defined a function that checks if a group of values are all equal. Then you could write:

    if all_equal(row1[0], row2[0], row3[0]):
        print("All %s's in top row!" % row1[0])
    

    That makes the conditions both shorter and easier to read.

  • Your script says “All X's in diagonal”, regardless of which diagonal it actually is. That’s less clear than it could be, and means it could potentially print the same sentence twice. I’d recommend using different terms to describe each diagonal – perhaps primary and secondary diagonal?

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