4
\$\begingroup\$

I want my code to write to a .csv file a bunch of patterns for Tic Tac Toe games (30000). I want to have 9 columns numbered 1 through 9 and in each have the number of the space in the grid that was played during that turn. Next a column with the entire sequence, then the moves the first player made, the moves the second player made and then the winner. My code is incredibly inefficient though, has been running for hours (I also need the second player to be somewhat intelligent but I can't use data from previous games to achieve this).

import csv
import random

class TotitoGeneratorManager:
    def __init__(self, path, games):
        self.path = path
        self.games = games
        self.winner = None
        self.player1 = []
        self.player2 = []
        self.gamesequence = []
        self.turns = 0
        self.gameover = False

    def game(self):
        while self.games > 0:
            while not self.gameover:
                while self.turns > 4:
                    player1play()
                    player2play()
                if self.turns > 4:
                    winner()
                    if self.gameover:
                        write_game()
                        self.games -= 1
                    else:
                        player1play()
                        player2play()


    def player1play(self):
        play = random.randint(0,9)
        played = False
        while not played:
            if play not in self.gamesequence:
                self.player1.append(play)
                self.gamesequence.append(play)
                played = True
                self.turns +=1
            else:
                play = random.randint(0,9)


    def player2play(self):
        played = False
        while not played:
            if 4 not in self.gamesequence:
                self.player2.append(4)
                self.gamesequence.append(4)
                played = True
                self.turns += 1
            elif 0 not in self.gamesequence:
                positions = [[1,2], [2,1], [3,6], [6,3], [4,8], [8,4]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(0)
                    self.gamesequence.append(0)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 1 not in self.gamesequence:
                positions = [[0,2], [2,0], [4,7], [7,4]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(1)
                    self.gamesequence.append(1)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 2 not in self.gamesequence:
                positions = [[0,1], [1,0], [5,8], [8,5], [6,4], [4,6]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(2)
                    self.gamesequence.append(2)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 3 not in self.gamesequence:
                positions = [[0,6], [6,0], [4,5], [5,4]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(3)
                    self.gamesequence.append(3)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 5 not in self.gamesequence:
                positions = [[2,8], [8,2], [3,4], [4,3]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(3)
                    self.gamesequence.append(3)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 6 not in self.gamesequence:
                positions = [[0,3], [3,0], [7,8], [8,7], [4,2], [2,4]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(3)
                    self.gamesequence.append(3)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 7 not in self.gamesequence:
                positions = [[6,8], [8,6], [1,4], [4,1]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(3)
                    self.gamesequence.append(3)
                    played = True
                    self.turns +=1
                else:
                    break
            elif 8 not in self.gamesequence:
                positions = [[2,5], [5,2], [0,4], [6,7], [7,6]]
                if self.player1 in positions or self.player2 in positions:
                    self.player2.append(3)
                    self.gamesequence.append(3)
                    played = True
                    self.turns +=1
                else:
                    break
            else:
                play = random.randint(0,9)
                played2 = False
                while not played2:
                    if play not in self.gamesequence:
                        self.player2.append(play)
                        self.gamesequence.append(play)
                        played = True
                        played2 = True
                        self.turns +=1
                    else:
                        play = random.randint(0,9)

    def winner(self):
        winning_positions = [[0,1,2], [3,45], [6,7,8], [0,3,6], [1,4,7], [2,5,8], [0,4,8], [2,4,6]]
         if self.player1 in winning_positions:
            self.winner == 1
            self.gameover = True
        elif self.player2 in winning_positions:
            self.winner == 1
            self.gameover = True
        elif self.turns == 9:
            self.winner = 'E'
            self.gameover = True

    def create_csv(self):
        with open(self.path, 'wb') as write_file:
            header = [[1, 2, 3, 4, 5, 6, 7, 8, 9, "Secuencia", "Tiros A", "Tiros B", "Ganador"]]
            for row in header:
                for column in row:
                    write_file.write('%s,' % column)
                write_file.write('\n')

    def write_game(self):
        with open(self.path, 'wb') as write_file:
            for column in self.gamesequence:
                write_file.write('%s,' % column+1)
            write_file.write(self.gamesequence, ',')
            write_file.write(self.player1, ',')
            write_file.write(self.player2, ',')
            write_file.write(self.winner, '.')
            write_file.write('\n')
            write_file.close()

def main():
    path = "/Users/mtchavez/Assignments/CSFundamentalsUFM/totito.csv"
    games = 30000 #number of games to generate
    tgm = TotitoGeneratorManager(path, games)
    tgm.create_csv()
    tgm.game()

main()
\$\endgroup\$
3
\$\begingroup\$

Duplicated code blocks

There is an awful lot of repetition here. For example this kind of code:

if self.player1 == [1,2] or self.player1 == [2,1] or self.player1 == [3,6] or self.player1 == [6,3] or self.player1 == [4,8] or self.player1 == [8,4]:
    self.player2.append(0)
    self.gamesequence.append(0)
    played = True
    self.turns +=1
elif self.player2 == [1,2] or self.player2 == [2,1] or self.player2 == [3,6] or self.player2 == [6,3] or self.player2 == [4,8] or self.player2 == [8,4]:
    self.player2.append(0)
    self.gamesequence.append(0)
    played = True
    self.turns +=1

The branches of the if-else have the same code, so this could be a single if without elif, and without the duplication.

Then, the two sets of conditions are also very similar, first checking if player1 is one of the combinations, or if player2 is one of the combinations. A shorter way of writing the exact same thing:

positions = [[1,2], [2,1], [3,6], [6,3], [4,8], [8,4]]
if self.player1 in positions or self.player2 in positions:
    self.player2.append(0)
    self.gamesequence.append(0)
    played = True
    self.turns +=1

Review the rest of the code, and try to eliminate duplicated code in a similar fashion.

while x == False

The Pythonic way to write this:

while not x:

Use with to auto-close file handles

When working with file handles, it's recommended to use with, for example given this code:

write_file = open(self.path, 'wb')
header = [[1, 2, 3, 4, 5, 6, 7, 8, 9, "Secuencia", "Tiros A", "Tiros B", "Ganador"]]
for row in header:
    for column in row:
        write_file.write('%s,' % column)
    write_file.write('\n')
write_file.close()

This is cleaner:

with open(self.path, 'wb') as write_file:
    header = [[1, 2, 3, 4, 5, 6, 7, 8, 9, "Secuencia", "Tiros A", "Tiros B", "Ganador"]]
    for row in header:
        for column in row:
            write_file.write('%s,' % column)
        write_file.write('\n')

This way you don't need to worry about closing write_file, Python will do it for you automatically.

\$\endgroup\$
  • \$\begingroup\$ I tried making these improvements and the code is still taking forever to run (tried to run it to generate only 5 sequences to test it out) \$\endgroup\$ – user3089079 Feb 20 '15 at 14:54
2
\$\begingroup\$

player2play seems like a really long method, so that's where improvements should be focussed.

First, you have lots of code blocks that are simple duplicates of eachother; deduplicate these. I suggest using janos' techinque. This gives:

def player2play(self):
    def gamesequence 

    played = False
    while played == False:
        if 4 not in self.gamesequence:
            self.player2.append(4)
            self.gamesequence.append(4)
            played = True
            self.turns += 1
        elif 0 not in self.gamesequence:
            positions = [1,2], [2,1], [3,6], [6,3], [4,8], [8,4]
            if self.player1 in positions or self.player2 in positions:
                self.player2.append(0)
                self.gamesequence.append(0)
                played = True
                self.turns +=1
            else:
                break
        elif 1 not in self.gamesequence:
            positions = [0,2], [2,0], [4,7], [7,4]
            if self.player1 in positions or self.player2 in positions:
                self.player2.append(1)
                self.gamesequence.append(1)
                played = True
                self.turns +=1
            else:
                break

        # and so on ...

        else:
            play = random.randint(0,9)
            played2 = False
            while played2 == False:
                if play not in self.gamesequence:
                    self.player2.append(play)
                    self.gamesequence.append(play)
                    played = True
                    played2 = True
                    self.turns +=1
                else:
                    play = random.randint(0,9)

This is just the first step; look at the differences in each and extract this into a loop:

def player2play(self):
    played = False
    while played == False:
        positionss = (
            ([1, 2], [2, 1], [3, 6], [6, 3], [4, 8], [8, 4]),
            ([0, 2], [2, 0], [4, 7], [7, 4]),
            ([0, 1], [1, 0], [5, 8], [8, 5], [6, 4], [4, 6]),
            ([0, 6], [6, 0], [4, 5], [5, 4]),
            None, # 4 always matches
            ([2, 8], [8, 2], [3, 4], [4, 3]),
            ([0, 3], [3, 0], [7, 8], [8, 7], [4, 2], [2, 4]),
            ([6, 8], [8, 6], [1, 4], [4, 1]),
            ([2, 5], [5, 2], [0, 4], [4, 0], [6, 7], [7, 6]),
        )

        plays = 0, 1, 2, 3, 4, 3, 3, 3, 3

        for i in range(9):
            if i not in self.gamesequence:
                positions = positionss[i]
                play = plays[i]

                if i == 4 or self.player1 in positions or self.player2 in positions:
                    self.player2.append(play)
                    self.gamesequence.append(play)
                    played = True
                    self.turns += 1

                # Exit
                return

        # If none of the above triggered
        play = random.randint(0,9)
        played2 = False
        while played2 == False:
            if play not in self.gamesequence:
                self.player2.append(play)
                self.gamesequence.append(play)
                played = True
                played2 = True
                self.turns +=1
            else:
                play = random.randint(0,9)

played doesn't seem to be useful either; if the if in the for is ever hit the loop will be exited by the return and if not, the while played2 == False will only exit after played = True anyway. Therefore we can have just:

def player2play(self):
    positionss = (
        ([1, 2], [2, 1], [3, 6], [6, 3], [4, 8], [8, 4]),
        ([0, 2], [2, 0], [4, 7], [7, 4]),
        ([0, 1], [1, 0], [5, 8], [8, 5], [6, 4], [4, 6]),
        ([0, 6], [6, 0], [4, 5], [5, 4]),
        None, # 4 always matches
        ([2, 8], [8, 2], [3, 4], [4, 3]),
        ([0, 3], [3, 0], [7, 8], [8, 7], [4, 2], [2, 4]),
        ([6, 8], [8, 6], [1, 4], [4, 1]),
        ([2, 5], [5, 2], [0, 4], [4, 0], [6, 7], [7, 6]),
    )

    plays = 0, 1, 2, 3, 4, 3, 3, 3, 3

    for i in range(9):
        if i not in self.gamesequence:
            positions = positionss[i]
            play = plays[i]

            if i == 4 or self.player1 in positions or self.player2 in positions:
                self.player2.append(play)
                self.gamesequence.append(play)
                self.turns += 1

            # Exit
            return

    # If none of the above triggered
    play = random.randint(0,9)
    while True:
        if play not in self.gamesequence:
            self.player2.append(play)
            self.gamesequence.append(play)
            self.turns += 1
            return
        else:
            play = random.randint(0,9)

The final loop can deduplicate the play = random.randint(0, 9):

    # If none of the above triggered
    while True:
        play = random.randint(0, 9)

        if play not in self.gamesequence:
            self.player2.append(play)
            self.gamesequence.append(play)
            self.turns += 1
            return

You can remove duplication between it and the loop using a function capturing the loop body:

def player2play(self):
    positionss = (
        ([1, 2], [2, 1], [3, 6], [6, 3], [4, 8], [8, 4]),
        ([0, 2], [2, 0], [4, 7], [7, 4]),
        ([0, 1], [1, 0], [5, 8], [8, 5], [6, 4], [4, 6]),
        ([0, 6], [6, 0], [4, 5], [5, 4]),
        None, # 4 always matches
        ([2, 8], [8, 2], [3, 4], [4, 3]),
        ([0, 3], [3, 0], [7, 8], [8, 7], [4, 2], [2, 4]),
        ([6, 8], [8, 6], [1, 4], [4, 1]),
        ([2, 5], [5, 2], [0, 4], [4, 0], [6, 7], [7, 6]),
    )

    plays = 0, 1, 2, 3, 4, 3, 3, 3, 3

    def get_play():
        for i in range(9):
            if i not in self.gamesequence:
                positions = positionss[i]

                if i == 4 or self.player1 in positions or self.player2 in positions:
                    return plays[i]

                # Don't make a move
                return

        # If none of the above triggered
        while True:
            play = random.randint(0, 9)
            if play not in self.gamesequence:
                return play

    play = get_play()
    if play is not None:
        self.player2.append(play)
        self.gamesequence.append(play)
        self.turns += 1

This makes it clearer that we have:

    for i in range(9):
        if i not in self.gamesequence:
            ...
            return

    # If none of the above triggered
    while True:
        play = random.randint(0, 9)
        if play not in self.gamesequence:
            ...

Surely the if in the while True can only hold for play = 9. You might as well add that as another case in the loop instead:

def player2play(self):
    positionss = (
        ([1, 2], [2, 1], [3, 6], [6, 3], [4, 8], [8, 4]),
        ([0, 2], [2, 0], [4, 7], [7, 4]),
        ([0, 1], [1, 0], [5, 8], [8, 5], [6, 4], [4, 6]),
        ([0, 6], [6, 0], [4, 5], [5, 4]),
        None, # 4 always matches
        ([2, 8], [8, 2], [3, 4], [4, 3]),
        ([0, 3], [3, 0], [7, 8], [8, 7], [4, 2], [2, 4]),
        ([6, 8], [8, 6], [1, 4], [4, 1]),
        ([2, 5], [5, 2], [0, 4], [4, 0], [6, 7], [7, 6]),
        None, # 9 always matches
    )

    plays = 0, 1, 2, 3, 4, 3, 3, 3, 3, 9

    for i in range(10):
        if i not in self.gamesequence:
            positions = positionss[i]
            play = plays[i]

            if positions is None or self.player1 in positions or self.player2 in positions:
                self.player2.append(play)
                self.gamesequence.append(play)
                self.turns += 1

            return

    raise RuntimeError("Impossible state")

It's worth noting that the whole plays != i thing is confusing - why is it 0, 1, 2, 3, 4, 3, 3, 3, 3, 9? The lack of documentation on why 4 is special-cased is confusing too.

I have a feeling at least some of these are bugs in the program.


Note: This code is untested and may be wrong.

\$\endgroup\$
  • \$\begingroup\$ I tried making these improvements and the code is still taking forever to run (tried to run it to generate only 5 sequences to test it out) \$\endgroup\$ – user3089079 Feb 20 '15 at 14:50
  • \$\begingroup\$ @user3089079 It would help if you answered the questions I raised, such as "why is it 0, 1, 2, 3, 4, 3, 3, 3, 3, 9?" The problem is mostly that I don't get how the code is meant to work, so it's hard to tell you what to change. What was the while at the end meant to do? - it obviously wasn't working as expected! Why is 4 special-cased? What even is the function doing? What's the meaning of the specific value of positionss? \$\endgroup\$ – Veedrac Feb 20 '15 at 16:47
  • \$\begingroup\$ Positions are just the list of tic tac toe positions in which you only need the number in said code block to reach a winning position. 4 is a special case because it is strategically the best number you can start off with (so if the first player did not start off with 4 you should). I am unsure where you got 0,1,2,3,4,3,3,3,3,9 from \$\endgroup\$ – user3089079 Feb 21 '15 at 18:03

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.