HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Making Tic-Tac-Toe winning check more efficient

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
checktoeticmoreefficienttacwinningmaking

Problem

My code works perfectly fine and I would just like to clean it up a little. From the research I've done, I cannot find anyone that used a multidimensional list to a game of TTT. I'm just looking for a way to make this code prettier than it is right now.

```
grid = [['-','-','-'],
['-','-','-'],
['-','-','-']]
turn = 'x'
stl = True
print("Please use this format: row/col")
rounds = 0

def printGrid(grid):
for i,j,k in grid:
b = print(i,j,k)

def place(grid,turn,row,col):
if grid[row][col] == '-':
grid[row][col] = turn
return True

else:
print("Please select an untaken postion\n")
return False

while stl:
rounds += 1
print("Round %i!" % rounds)

try:

printGrid(grid)
print("\nIt is %s's turn.\n" % turn)
inp = input("What position do you want to place your symbol? (row/col 1-3/1-3) ")
row,col = inp.split('/')
row = int(row)-1
col = int(col)-1
print()

a = place(grid,turn,row,col)

if a == True:
if turn == 'x':
turn = 'o'
else:turn = 'x'

except(IndexError,ValueError):print("\nPlease select a viable position.\n")

print('----------------------------------\n')

if ['x','x','x'] in grid:
print("X Victory! You won in %i rounds!\n" % rounds)
printGrid(grid)
stl = False
elif ['o','o','o'] in grid:
print("O Victory! You won in %i rounds!\n" % rounds)
printGrid(grid)
stl = False

if grid[0][0] == 'x' and grid[1][0] == 'x' and grid[2][0] == 'x':
print("X Victory! You won in %i rounds!" % rounds)
printGrid(grid)
stl = False
elif grid[0][1] == 'x' and grid[1][1] == 'x' and grid[2][1] == 'x':
print("X Victory! You won in %i rounds!" % rounds)
printGrid(grid)
stl = False
elif grid[0][2] == 'x' and grid[1][2] == 'x' and grid[2][2] == 'x':
print("X Victory! You won in

Solution

You could store the winning positions as follows:

winning = [[(0, 0), (1, 0), (2, 0)],
           [(0, 1), (1, 1), (2, 1)],
           [(0, 2), (1, 2), (2, 2)], 
           ...]


Now you can much more easily loop through the positions and test them, using all:

for positions in winning:
    for symbol in 'xy':
        if all(grid[r][c] == symbol for r, c in positions):
            print("{} Victory! You won in {} rounds!".format(symbol.upper(), 
                                                             rounds)
            stl = False
            break


I would wrap all of this in a class, to make life a bit easier:

class TicTacToe:

    BLANK = '-'

    WINNING = [[(0, 0), (1, 0), (2, 0)],
               [(0, 1), (1, 1), (2, 1)],
               [(0, 2), (1, 2), (2, 2)], 
               ...]

    def __init__(self):
        self.grid = [[self.BLANK for _ in range(3)] for _ in range(3)]

    def __str__(self):
        return '\n'.join([' '.join(row) for row in self.grid])

    def won(self):
        """Return the winning symbol, or None if not yet won."""
        for positions in self.WINNING:
            for symbol in 'xy':
                if all(self.grid[r][c] == symbol for r, c in positions):
                    return symbol

   def make_move(self, symbol, row, col):
       if self.grid[row][col] != self.BLANK:
           raise ValueError("Position already occupied.")
       self.grid[row][col] = symbol

   ...


In terms of general code review, note that:

  • You should not have multiple statements on the same line (e.g. else:turn = 'x');



  • The style guide includes guidance on whitespace (e.g. see the spaces after commas in the code I have posted);



  • There's no need to assign the result of print - it is None, just print(i,j,k);



  • You should not have code just running within the script, write a main function and guard a call to it behind if __name__ == '__main__':; and



  • You should generally have more, smaller blocks of functionality (e.g. you could have a function whose job is to get the user to input a valid row/col selection).

Code Snippets

winning = [[(0, 0), (1, 0), (2, 0)],
           [(0, 1), (1, 1), (2, 1)],
           [(0, 2), (1, 2), (2, 2)], 
           ...]
for positions in winning:
    for symbol in 'xy':
        if all(grid[r][c] == symbol for r, c in positions):
            print("{} Victory! You won in {} rounds!".format(symbol.upper(), 
                                                             rounds)
            stl = False
            break
class TicTacToe:

    BLANK = '-'

    WINNING = [[(0, 0), (1, 0), (2, 0)],
               [(0, 1), (1, 1), (2, 1)],
               [(0, 2), (1, 2), (2, 2)], 
               ...]

    def __init__(self):
        self.grid = [[self.BLANK for _ in range(3)] for _ in range(3)]

    def __str__(self):
        return '\n'.join([' '.join(row) for row in self.grid])

    def won(self):
        """Return the winning symbol, or None if not yet won."""
        for positions in self.WINNING:
            for symbol in 'xy':
                if all(self.grid[r][c] == symbol for r, c in positions):
                    return symbol

   def make_move(self, symbol, row, col):
       if self.grid[row][col] != self.BLANK:
           raise ValueError("Position already occupied.")
       self.grid[row][col] = symbol

   ...

Context

StackExchange Code Review Q#70493, answer score: 2

Revisions (0)

No revisions yet.