0
\$\begingroup\$

The previous code on codereview

I'm quite novice here so apologies for any silly mistake in advance

I've been writing a simple tic tac toe game which is a part of my course in udemy

Since this is my first project, I want to do my best in order to learn new things besides learning how to code better. Generally, I want to optimize my code as much as possible.

Can anyone help me with this?

# Tic Tac Toe
# 17 July 2020

import os

test_board = [' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ']
player_input_num = 0    # numbers of inputs entered by player
player_num = 0
won = False
marker = 'X'


def clear_screen():
    os.system("clear")


def display_board(board_cords):
    '''
    display the board
    board_cords = list(left to right from top to bottom)
    '''
    line = '-'*40
    for counter in range(0, len(board_cords), 3):
        print("\t|\t\t|")
        print(
            f"{board_cords[counter]}\t|\t{board_cords[counter+1]}\t|\t{board_cords[counter+2]}")
        print(line)
        counter += 3    # go to next row


def check_player_input(number):
    if number < 10 and number >= 0:  # checking the range
        return True
    else:
        print('Sorry the input is not in range [0-9] .')
        return False


def player_input(player_in):
    global player_input_num  # access to player_input_num
    if player_input_num >= 2:
        # check if the position is free to use
        if check_capacity(test_board, player_in):
            result = place_marker(test_board, 'X', player_in)
            clear_screen()
            display_board(result)

            if check_win(test_board, marker):
                print("You Won!")
                global won
                won = True
        else:
            print("The current position is occupied.")
    else:
        if check_capacity(test_board, player_in):
            result = place_marker(test_board, 'X', player_in)
            clear_screen()
            display_board(result)
            player_input_num += 1
        else:
            print("The current position is occupied.")


def check_capacity(board, position):
    '''
    Check if the current position is free to use.
    '''
    return board[position] == ' '


def place_marker(board, marker, position):
    '''
    Replace the position with a marker
    '''
    board[position] = marker
    return board


def check_win(board, marker):
    '''
    Check if the current game is finished
    '''
    if board[0] == board[1] == board[2] == marker:
        return True
    if board[0] == board[3] == board[6] == marker:
        return True
    if board[0] == board[4] == board[8] == marker:
        return True
    if board[2] == board[5] == board[8] == marker:
        return True
    if board[6] == board[7] == board[8] == marker:
        return True
    if board[2] == board[4] == board[6] == marker:
        return True
    return False


def wanna_play():
    answer = input("Wanna play again? (Y or N)")
    return answer


# Main
display_board(test_board)
while True:
    while won == False:
        try:  # checking if input is int
            player_num = int(input("Enter a position: "))
        except:
            print("Input is not a number")
            continue

        if check_player_input(player_num):
            player_input(player_num)

    if wanna_play() in ['y', 'Y', 'n', 'N']:
        print("something")
    else:
        print("Invalid input.")
        # print("Thanks for playing :)")

Note that code is working, I just want to know other ways of doing it and optimize it overalls.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Instead of using indices from 0 to 9, change the way you store your board to be a 3x3 list.

board = list([[0] * 3] * 3)
"""
1 | 2 | 3
4 | 5 | 6
7 | 8 | 9
"""

This will allow you to come back to it and add a GUI. It also becomes easier for anyone reading the code to understand what's happening visually.

Next is your printout of the board. Join will be your friend here.

for row in board:
  printout = "\t|\t".join(str(element) for element in row)
  print(printout) # this can be reduced to a single line if you want.

ranges

range() accepts 3 variables. START, STOP and INCREMENT AMOUNT

for i in range(0, 9, 3):

You can also reverse with negative values

for i in range(9, 0, -1):

or

for row in board[::-1]:

if statements

In math class you know how you were able to write a < x > b? You can do that in python without requiring and.

if 0 <= number > 10:

Then there is checking the board state if someone has won. Your current approach requires a lot of coding and can't be expanded on quickly

There are a few different approaches you can do with this. All will "work."

First the rows

for row in board:
  if len(set(row)) == 1:
    return True
for row in board:
  if all(element == row[0] for element in rows):
    return True

Diagonals next (I would use a tempory dummy variable for len(board) to increase readability. -> _ = len(board)

if len(set(board[i][i] for i in range(len(board)) == 1:
  return True
if len(set(board[i][len(board)-i-1] for i in range(len(board)) == 1:
  return True

The columns can be a trickier but transposing would probably be the most pythonic way to approach it.

board_T = [list(column) for column in zip(board)]

This will allow you to call the rows again and check to see if someone won.

It should be noted that the return values for the winner state could return the PLAYER of rather than True or False. Python sees 0, None, Empty iterables, and False as False in an if statement.

I also noticed you didn't follow a lot of the advice given to you in the original post. You are still lacking an if __name__ == "__main__": statement

def playTicTacToe():
   display_board(test_board)
   ...

...
if __name__ == "__main__":
  playTicTacToe()
\$\endgroup\$
8
  • \$\begingroup\$ Thank you so much for your amazing help. But now I faced a problem. When I changed my code and do everything before range in your answer, the output is : [0, 0, 0] | [0, 0, 0] | [0, 0, 0] Enter a position: \$\endgroup\$
    – Darklight
    Jul 17, 2020 at 22:16
  • \$\begingroup\$ And Yes. I searched for name thing but i just couldn't find how to use it in my code. \$\endgroup\$
    – Darklight
    Jul 17, 2020 at 22:18
  • \$\begingroup\$ @Darklight I added a bit to the end to help with the name == main bit. Also, I had a typo at board = it should be board = list([[0] * 3] * 3]), sorry about that \$\endgroup\$ Jul 17, 2020 at 23:08
  • \$\begingroup\$ @david_fisher Thanks for your help. I changed the test_board variable to test_board = list([[' '] * 3] * 3) but now the function check_capacity() always returns false \$\endgroup\$
    – Darklight
    Jul 18, 2020 at 18:13
  • \$\begingroup\$ You need to convert your position to y, x = position // 3, position % 3 and then run the check return board[y][x] == ' ' Btw, you'll also need to have these values for when you place your 'X' and 'O' \$\endgroup\$ Jul 18, 2020 at 18:50

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.