4
\$\begingroup\$

I'm a beginner to python and as part of my course I'm instructed to create a simple tic tac toe game.

I'd be very appreciative and interested of any insight, criticism, instructions, best practices or code readability and tips.

Are my comments descriptive, repetitive and useful enough?

Any inputs and tips would be greatly appreciated.

Here's my code

import random
#gives the apearance of rewriting the board on the screen
clear = lambda: print('\n' * 20)
#will be used as the game board; to hold the x's and o's and determine a win etc.
test_board = ['#', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ']# the '#' is at index 0 so that players can input 1 to use first position instead of 0


# prints testboard with lines so that it appears like a real board
def display_board(board):
    print(board[1] + '|' + board[2] + '|' + board[3])
    print('-|-|-')
    print(board[4] + '|' + board[5] + '|' + board[6])
    print('-|-|-')
    print(board[7] + '|' + board[8] + '|' + board[9])


# assigns x to player1 and o to player2
def player_input():
    marker = ''
    while not (marker == 'x' or marker == 'o'): #this loop goes over and over untill the playerinput is an x or o.
        marker = input('player1 please choose a marker. "x" or "o"')
    if marker == 'x':
        return ('x', 'o') # to assign x to player1 and o to player2 with tuple unpacking
    else:
        return ('o', 'x') # to assign o to player1 and x to player2 with tuple unpacking


# converts a players integer input to and assings either x or o to the testboard's index of the input
def place_marker(board, marker, position):
    board[position] = marker


# checks to board for a win
def win_check(board, mark):
    return ((board[1] == mark and board[2] == mark and board[3] == mark) or  # across top
            (board[4] == mark and board[5] == mark and board[6] == mark) or  # across middle
            (board[7] == mark and board[8] == mark and board[9] == mark) or  # across bottom
            (board[1] == mark and board[4] == mark and board[7] == mark) or  # down left
            (board[2] == mark and board[5] == mark and board[8] == mark) or  # down middle
            (board[3] == mark and board[6] == mark and board[9] == mark) or  # down right
            (board[1] == mark and board[5] == mark and board[9] == mark) or  # diagonal upper right to lower left
            (board[3] == mark and board[5] == mark and board[7] == mark)  # diagonal upper left to lower right
            )


#chooses a player to go first
def choose_first():
    random_int = random.randint(1, 10)
    if random_int % 2 == 0:
        print('player1 may go first')
        return True
    else:
        print('player2 may go first')
        return False


# when player gives index position, checks if index position is not already taken
def space_check(board, position):
    return board[position] == ' '


# checks if the board is full
def full_board_check(board):
    for x in range(1, 10):
        if space_check(board, x):
            return False #the index position of the player's input is ' ' based on space_check so its False. i.e. empty
    return True #the index position of the player's input is not ' ' based on space_check so its True. i.e. full


# accepts the players position for where he wants to go
def player_choice(board):
    while True:
        a = input('please input an integer 1-9')#will be the index position of test_board
        try:
            a = int(a)#must convert to int to be used in test_board indexing
        except ValueError:#ensures that the players input is an integer
            clear()
            print('\n please type a number')
            print('_____________________')
            display_board(board)
            continue
        if a >= 1 and a <= 9:#since there are only nine positions in test_board, the players input must be between 1 and 9
            if space_check(board, a):
                return a #return a(player's input) to be used in test_board
            elif a < 1 or a > 9:
                print('please input an integer from 1-9')
            else:
                clear()
                print('\n that space isn\'t available')
                print('____________________________')
                display_board(board)
                continue


# when game is over, replay the game
def replay():
    replay = input('press any key to play again. type "q" to quit')
    while True:
        if 'q' in replay:
            return False
            break
        else:
            return True
            break


while True:
    #set the game up
    test_board = ['#', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' '] #at the begining of the game empty's the board.
    #randomly select a player to go first
    if choose_first():
        player1,player2=player_input()
    else:
        player2,player1=player_input()
    game_on=True
    #players take turns
    while game_on:
        #player1's turn
        display_board(test_board)
        print('%s may go'% player1)
        place_marker(test_board,player1,player_choice(test_board))#assings the players input to test_board
        if win_check(test_board,player1):
            display_board(test_board)
            print('%s has won! \n game over'% player1)
            break #breaks out of local loop and jumps to next loop (if not replay())
        elif full_board_check(test_board):
            display_board(test_board)
            print('board is full')
            break
        #player2's turn
        print('%s may go' % player2)
        display_board(test_board)
        place_marker(test_board,player2,player_choice(test_board))
        if win_check(test_board,player2):
            display_board(test_board)
            print('%s has won! \n game over'% player2)
        elif full_board_check(test_board):
            display_board(test_board)
            print('board is full')
            break
    if not replay():
        break




\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

use platform-independent clear function

from os import system, name
def clear():
    if name == 'nt':
        system('cls')
    else:
        system('clear')

This new function clear is platform-independent and slightly more elegant than the approach you used.

Notes on Comments

Comments are good but a necessary evil, since you highlighted that you are doing a course, it might be a requirement to include "descriptive comments", but you won't be doing that course all your life. Comments should be avoided as much as possible. Use comment to explain intent(why something was done) not how it was done. If a thought to include a comment crosses your mind, ask yourself, "How would I express myself in code?"

Consider using a 2D to represent a board

Your internal representation of a board is a 1D list. This is fine, but I would advise you to mirror a real-life application, a tic-tac-toe game is mostly 2D or 3D, a 2D representation would be

board = [ [ ' ' for _ in range(3)] for _ in range(3) ]

Accessing this is also easy with just an index

index = 5 # an example
row = index // len(board)
col = index % len(board)

board[row][col] = marker

Improvement on names and structure

Right now, your code is messy and hard to read, win_check is a little confusing, a nice and better structure might be something like this

def check_victory():
    if row_victory():
        return True
    if column_victory():
        return True
    if diagonal_victory():
        return True

    return False

From the look of this, it is quite easy to follow what is going on here. row_victory, column_victory, diagonal_victory might be implemented as follows

def row_victory():
    for i in range(len(board)):
        if (board[i][0] != ' ' and
            board[i][0] == board[i][1] == board[i][2]):
                return True

    return False

def column_victory():
    for i in range(len(board)):
        if (board[0][i] != ' ' and
            board[0][i] == board[1][i] == board_array[2][i]):
                return True

    return False

def diagonal_victory():
    if (board[0][0] != ' ' and
        board[0][0] == board[1][1] == board[2][2]):
        return True
    if (board[0][2] != ' ' and
        board[0][2] == board[1][1] == board[2][0]):
        return True

    return False

This would mean you need to keep a current_player variable, this should seem easy for you. When any of the above function returns true, the current player is the winner. Consider renaming full_board_check to stalemate.

Names like a should be avoided, a better alternative is user_input

Consider using a class

Using a class would really improve the structure and readability of your code

\$\endgroup\$
5
  • \$\begingroup\$ This is a very nice review, But I don't agree with the 2-dimensional board thing, You will always need two subscripts to access as square, always keep two variables to represent a position, and will always have to convert the user input to your way of representing the board. A lot of extra lines of code, for what? \$\endgroup\$
    – user228914
    Commented Nov 26, 2020 at 17:56
  • 1
    \$\begingroup\$ Also, from your check_victory() function. return row_victory() or col_victory() or diag_victory() :) \$\endgroup\$
    – user228914
    Commented Nov 26, 2020 at 18:10
  • \$\begingroup\$ @Aryan Parekh, A 2d board mirrors tic-tac-toe real life implementation, the overhead of having two variables to represent a position is negligible \$\endgroup\$ Commented Nov 26, 2020 at 19:30
  • \$\begingroup\$ Real-life implementation? How so, in my head I see 1-9, that's why you ask the user to enter a number from 1-9. \$\endgroup\$
    – user228914
    Commented Nov 26, 2020 at 19:31
  • \$\begingroup\$ I mean't the board is represented on a 2d plane with x and y co-ordinates. \$\endgroup\$ Commented Nov 26, 2020 at 19:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.