My first project in Python: Tic Tac Toe

Question

I just finished coding a Tic Tac Toe game for my first Python project. I need some advice on things I can improve; please help. Everything seems to be running correctly. I used many functions. In the comments you can know what functions do; there is board initialization and prints the board out to the console checks the input and updates the board according to the user's decision, function analyzes the board status in order to check the player using 'O's or 'X's has won the game and a function draws the computer's move and updates the board.

from random import randrange
result = False
board=[[1,2,3],[4,'X',6],[7,8,9]]
#
#board initialization always first move of computer is in the middle

def DisplayBoard(board):
    for j in range(4):
        for i in range(4):
            print("+",end='')
            if i==3:
                break
            for i in range(7):
                print("-",end='')
        if j==3:
            break
        print()
        for d in range (3):
            for r in range(4):
                print("|",end='')
                if r==3:
                    break
                for i in range(7):
                    if d==1 and i==3:
                        print(board[j][r],end='')
                    else:
                        print(" ",end='')
            print()
    print()
#
# the function accepts one parameter containing the board's current status
# and prints it out to the console
#
def EnterMove(board):
    entredMove=int((input("Enter your move: ")))
    while not any(entredMove in i for i in board):
            print ("this value is wrong")
            entredMove=int((input("Enter your move: ")))
    for i in range(3):
        for j in range(3):
            if int(entredMove)==board[i][j]:
                board[i][j]= 'O'
#
# the function accepts the board current status, asks the user about their move, 
# checks the input and updates the board according to the user's decision
#

def MakeListOfFreeFields(board):
    freeFields=[]
    s=0
    global result
    for i in range(3):
        for j in range(3):
            if type(board[i][j])== int:
                freeFields.append((i,j))
                s+=1
    if s==0 and result==False:
        result = True
        print ("it is a DRAW")    
                    

# the function browses the board and builds a list of all the free squares; 
# the list consists of tuples, while each tuple is a pair of row and column numbers
# and test if list is empty that means it is a draw

def VictoryFor(board, sign):
    global result
    xxx=0
    xxxx=0
    for i in range(3):
        x=0
        for j in range(3):
            if board[i][j]==sign:
                x+=1
                if x==3:
                    print(sign,' is won the game')
                    result=True
        if result == True:
            break
        xx=0
        for j in range(3):
            if board[j][i]==sign:
                xx+=1
                if xx==3:
                    print(sign,' is won the game')
                    result=True
        if result == True:
            break
        for j in range(3):
            if i==j and board[i][j]==sign:
                xxx+=1
                if xxx==3:
                    print(sign,' is won the game')
                    result=True
        if result ==True:
            break
        for j in range(3):
            if i+j==2 and board[i][j]==sign:
                xxxx+=1
                if xxxx==3:
                    print(sign,' is won the game')
                    result=True
        if result ==True:
            break
#
# the function analyzes the board status in order to check if 
# the player using 'O's or 'X's has won the game 
#

def DrawMove(board):
    entredMove=randrange(8)+1
    while not any(entredMove in i for i in board):
        entredMove=randrange(9)+1
    for i in range(3):
        for j in range(3):
            if int(entredMove)==board[i][j]:
                print('computer move in ',entredMove)
                board[i][j]= 'X'
#
# the function draws the computer's move and updates the board
#
DisplayBoard(board)
while result == False:
    EnterMove(board)
    DisplayBoard(board)
    VictoryFor(board, 'O')
    if result == False:
        DrawMove(board)
        VictoryFor(board, 'X')
        DisplayBoard(board)
        MakeListOfFreeFields(board)

Answer 1

Welcome to the community. Here are few pointers from first look at the code:

1. The code does not follow PEP-8 style guide. You should follow the snake_case naming convention for variables and functions; classes follow the CamelCase naming.

2. Instead of the code running as is, the execution condition should be placed inside an if __name__ block.

3. For comparing True, False, None; instead of == the is comparison check is preferred. So, instead of result == False, it would be result is False or just not result.

4. If using python 3.6+, you can also provide type hinting.

5. Instead of comments around the function definitions, use docstrings.

6. The following print statement(s) have a redundant loop (unnecessary CPU instructions):

    for i in range(7):
        print("-",end='')
   

   and can simply be:

    print("-" * 7)
   

Answer 2

Welcome to the community, my first post was also a tictactoe, allthough in a functional Style in Scala.

So first some general rules / suggestions, then later I will go into the details. If I am wrong on anything then please feel free to correct me. I am more experienced in Java and allthough I have done some projects in Python I still might be ignorant about how things are different.

1. Code should be readable from top to bottom
2. Try to indent as little as possible. (There are often more than 5 indentations)
3. Snake Case in Python so MakeListOfFreeFiels = make_list_of_free_fields
4. Use meaningful Variable names. (Your loops are hard to read i,j,r,s etc)
5. Use comments only when needed. Your names should be expressive enough

So first I would make play_game() function, so you have a clean interface to start your game. You just put everything you have in the bottom in it, and place it at the top, so it's more readable.

Ok on the VictoryFor() function:

The general pattern is pretty interesting. I wouldn't have thought about the way you solved it kind of algorhitmically.

Personally I solved this by defining a set of sets of all win patterns, then check if the values in those indexes are all the same. I used a flat array, you used a map, so if you want to try implementing it you need to change that. (Copied from scala but the idea is the same).

  val patterns: Set[Set[Int]] = Set(
    Set(0, 1, 2),
    Set(3, 4, 5),
    Set(6, 7, 8),
    Set(0, 3, 6),
    Set(1, 4, 7),
    Set(2, 5, 8),
    Set(0, 4, 8),
    Set(2, 4, 6)
  )

Now back to your implementation and some suggestions. You can represent the string in a different format. Just some syntactic sugar.

print(sign,' is won the game')
#changed to
print(f'{sign} has won the game')

To make your intent more clear you could split the loops up in separate functions. def check_hor_winner, def check_vert_winner, def check_diag_winner

Also I would rename sign to player.

If you changed victoryFor, so that it returns true or false, then you can remove those result = True and breaks, and just return True.

Here is the final changed VictoryFor function in your algorhitmic Style. Especially in the diagonal functions I would have probably just put in the hardcoded patterns but if you would make a 100x100 ticTacToe then it would make sense.

def has_won(board, player):
    if (
        has_won_vertically(board, player) or 
        has_won_horizontally(board, player) or 
        has_won_diagonal_1(board, player) or 
        has_won_diagonal_2(board, player)):
        return True
    return False


def has_won_vertically(board, player):
    for row in range(3):
        player_count = 0
        for column in range(3):
            if board[row][column] == player:
                player_count += 1
        if player_count == 3:
            return True
    return False

def has_won_horizontally(board, player):
    for column in range(3):
        player_count = 0
        for row in range(3):
            if board[row][column] == player:
                player_count += 1
        if player_count == 3:
            return True
    return False

def has_won_diagonal_1(board, player):
    player_count = 0
    for row in range(3):
        for column in range(3):
            if row == column and board[row][column] != player:
                return False
    return True

def has_won_diagonal_2(board, player):
    player_count = 0
    for row in range(3):
        for column in range(3):
            if row+column == 2 and board[row][column] != player:
                return False
    return True

Next up your MakeListOfFreeFields The function Name does not represent what it is doing. Making a List of the free fields is just an implementation Detail. What it actually is doing is checking if it is a draw. To reflect that let's rename it to is_draw, and while we're at it let's also remove the global variable result and make is_draw return True or false.

DrawMove and EnterMove can also be renamed to enter_move_player() and enter_move_computer. I'm still not satisfied completely with the names but it's more clear.

Here is the final result I made. There are still a lot of improovements possible but my time is running out. I'm open to any criticism

from random import randrange
board=[[1,2,3],[4,'X',6],[7,8,9]]
#
#board initialization always first move of computer is in the middle

def play_game():
    display_board(board)
    won = False
    draw = False
    while won == False and draw == False:
        enter_move_player(board)
        display_board(board)
        won = has_won(board, 'O')
        if won == False:
            enter_move_computer(board)
            won = has_won(board, 'X')
            display_board(board)
            draw = is_draw(board)

def display_board(board):
    for j in range(4):
        for i in range(4):
            print("+",end='')
            if i==3:
                break
            for i in range(7):
                print("-",end='')
        if j==3:
            break
        print()
        for d in range (3):
            for r in range(4):
                print("|",end='')
                if r==3:
                    break
                for i in range(7):
                    if d==1 and i==3:
                        print(board[j][r],end='')
                    else:
                        print(" ",end='')
            print()
    print()


def enter_move_player(board):
    enteredMove=int((input("Enter your move: ")))
    while not any(enteredMove in i for i in board):
            print ("this value is wrong")
            enteredMove=int((input("Enter your move: ")))
    for i in range(3):
        for j in range(3):
            if int(enteredMove)==board[i][j]:
                board[i][j]= 'O'


def is_draw(board):
    freeFields=[]
    s=0
    for i in range(3):
        for j in range(3):
            if type(board[i][j])== int:
                freeFields.append((i,j))
                s+=1
    if s==0 and result==False:
        print ("it is a DRAW")  
        return True
    return False
                    

def has_won(board, player):
    if (
        has_won_vertically(board, player) or 
        has_won_horizontally(board, player) or 
        has_won_diagonal_1(board, player) or 
        has_won_diagonal_2(board, player)):
        return True
    return False


def has_won_vertically(board, player):
    for row in range(3):
        player_count = 0
        for column in range(3):
            if board[row][column] == player:
                player_count += 1
        if player_count == 3:
            return True
    return False

def has_won_horizontally(board, player):
    for column in range(3):
        player_count = 0
        for row in range(3):
            if board[row][column] == player:
                player_count += 1
        if player_count == 3:
            return True
    return False

def has_won_diagonal_1(board, player):
    player_count = 0
    for row in range(3):
        for column in range(3):
            if row == column and board[row][column] != player:
                return False
    return True

def has_won_diagonal_2(board, player):
    player_count = 0
    for row in range(3):
        for column in range(3):
            if row+column == 2 and board[row][column] != player:
                return False
    return True

def enter_move_computer(board):
    enteredMove = randrange(8)+1
    while not any(enteredMove in i for i in board):
        enteredMove=randrange(9)+1
    for i in range(3):
        for j in range(3):
            if int(enteredMove)==board[i][j]:
                print('computer move in ',enteredMove)
                board[i][j]= 'X'


play_game()