3
\$\begingroup\$

I am writing Python code for a Tic Tac Toe game. I need to write a function that takes in three inputs: board, x, and y. Board being the current display of the board and then x and y being values of 0, 1, or 2. The game is set up to ask the user for coordinates.

def CheckVictory(board, x, y):

    #check if previous move was on vertical line and caused a win
    if board[0][y] == ('X') and board[1][y] == ('X') and board [2][y] == ('X'):
        return True
    if board[0][y] == ('O') and board[1][y] == ('O') and board [2][y] == ('O'):
        return True

    #check if previous move was on horizontal line and caused a win
    if board[x][0] == ('X') and board[x][1] == ('X') and board [x][2] == ('X'):
        return True
    if board[x][0] == ('O') and board[x][1] == ('O') and board [x][2] == ('O'):
        return True

    #check if previous move was on the main diagonal and caused a win
    if board[0][0] == ('X') and board[1][1] == ('X') and board [2][2] == ('X'):
        return True
    if board[0][0] == ('O') and board[1][1] == ('O') and board [2][2] == ('O'):
        return True
    #check if previous move was on the secondary diagonal and caused a win
    if board[0][2] == ('X') and board[1][1] == ('X') and board [2][0] == ('X'):
        return True
    if board[0][2] == ('O') and board[1][1] == ('O') and board [2][0] == ('O'):
        return True

    return False 
#end of CheckVictory function

The function is called in the game loop like so:

p_x, p_y = playerTurn(board)    #let player take turn
displayBoard(board)             #show board after move has been made
if CheckVictory(board, p_x, p_y):   #see if user has won
    print("CONGRATULATIONS, you win!")
    newGame(board)  #game over start new one
    continue

and it's similar for the computer turn.

I feel like there is a better way to write this function. I feel like I should be using x and y more or there is a better way to check rather than writing all the possibilities. What's a better way to write this to make it short and concise?

\$\endgroup\$
4
\$\begingroup\$

You know that a mark has been placed at board[x][y]. Then you only need this to check for a win on vertical line y:

if board[0][y] == board[1][y] == board [2][y]

Your comments state "check if previous move was on the main/secondary diagonal", but you don't actually check. You can use the expressions x == y and x + y == 2 to check that.

Simplified code:

def CheckVictory(board, x, y):

    #check if previous move caused a win on vertical line 
    if board[0][y] == board[1][y] == board [2][y]:
        return True

    #check if previous move caused a win on horizontal line 
    if board[x][0] == board[x][1] == board [x][2]:
        return True

    #check if previous move was on the main diagonal and caused a win
    if x == y and board[0][0] == board[1][1] == board [2][2]:
        return True

    #check if previous move was on the secondary diagonal and caused a win
    if x + y == 2 and board[0][2] == board[1][1] == board [2][0]:
        return True

    return False             
\$\endgroup\$
2
\$\begingroup\$

I would start by removing duplication. If you pass in the mark that the player being checked is using, then you can eliminate 1/2 your code.

def CheckVictory(board, x, y, mark):

    if board[x][0] == (mark) and board[x][1] == (mark) and board [x][2] == (mark):
           return True

    if board[0][y] == (mark) and board[1][y] == (mark) and board [2][y] == (mark):
           return True

    #check if previous move was on the main diagonal and caused a win
    if board[0][0] == (mark) and board[1][1] == (mark) and board [2][2] == (mark):
           return True

    #check if previous move was on the secondary diagonal and caused a win
    if board[0][2] == (mark) and board[1][1] == (mark) and board [2][0] == (mark):
           return True    

    return False 

#end of CheckVictory function

Please excuse me if I have syntax wrong, I've never used python before.

\$\endgroup\$
  • \$\begingroup\$ ...or set mark = board[x][y] inside the function. \$\endgroup\$ – Janne Karila Apr 9 '13 at 7:08
1
\$\begingroup\$

This solution will check horizontal lines, vertical lines and diagonal lines for a winner and return the player number. I just used player numbers 1, 2 instead of 'x', 'o' to avoid numpy array conversions.

board = np.empty((BOARD_SIZE,BOARD_SIZE))
winner_line = [
    np.array([1, 1, 1]),
    np.array([2, 2, 2])
]

def CheckVictory(board):
    for idx in range(BOARD_SIZE):
        row = board[idx, :]
        col = board[:, idx]
        diagonal_1 = np.diagonal(board)
        diagonal_2 = np.diagonal(np.fliplr(board))

        # Check for each player
        for player in range(1,3):
            if np.all(row == winner_line[player-1]) \
                    or np.all(col == winner_line[player-1]) \
                    or np.all(diagonal_1 == winner_line[player-1]) \
                    or np.all(diagonal_2 == winner_line[player-1]):
                return player # There is a winner
    return False # No winner
\$\endgroup\$
0
\$\begingroup\$

For a start you could make your code twice smaller by removing the logic common to 'X' and to 'O'. Then, you can perform all the comparisons in one go.

def CheckVictory(board, x, y):
    playerSymbols=['X','O']

    #check if previous move was on vertical line and caused a win
    if (board[0][y] in playerSymbols) and board[0][y] == board[1][y] ==  board[2][y]:

    #check if previous move was on horizontal line and caused a win
    if (board[x][0] in playerSymbols) and board[x][0] == board[x][1] == board [x][2]:
        return True

    #check if previous move was on the main diagonal and caused a win
    if (board[0][0] in playerSymbols) and board[0][0] == board[1][1] == board [2][2]:

    #check if previous move was on the secondary diagonal and caused a win
    if (board[0][2] in playerSymbols) and board[0][2] == board[1][1] == board [2][0]:
        return True

    return False 
\$\endgroup\$
0
\$\begingroup\$

Notes:

  • CheckVictory: Idiomatic in Python is check_victory.

  • CheckVictory(board, x, y): I think you are mixing two things here, putting a value in the board and checking if someone won. Your function should be doing only one thing, checking if someone won on a given board.

  • A standard approach is to prepare all the data you need (here, the positions/coordinates to check) and leave the code as simple as possible.

I'd write:

positions_groups = (
    [[(x, y) for y in range(3)] for x in range(3)] + # horizontals
    [[(x, y) for x in range(3)] for y in range(3)] + # verticals
    [[(d, d) for d in range(3)]] + # diagonal from top-left to bottom-right
    [[(2-d, d) for d in range(3)]] # diagonal from top-right to bottom-left
)

def get_winner(board):
    """Return winner piece in board (None if no winner)."""
    for positions in positions_groups:
        values = [board[x][y] for (x, y) in positions]
        if len(set(values)) == 1 and values[0]:
            return values[0]

board = [
    ["X", "X", "O"],
    ["O", "X", "X"],
    ["O", "X", "O"],
]

print(get_winner(board)) # "X"
\$\endgroup\$
  • \$\begingroup\$ "I think you are mixing two things here" - no, it's just using the location of the last move (which must be part of any newly-created victory) to narrow down the scope of the search. \$\endgroup\$ – Mark Reed Sep 16 '14 at 19:35

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.