0
\$\begingroup\$

I made a very primitive Tic-Tac-Toe using Python and I would like a grade on how well I coded the game.
What code was there that could've been simpler or more efficient? And any advice you can give me?

board = ['-','-','-','-','-','-','-','-','-']
request = None
currentLetter = 'X'
win = False

def displayBoard():
    print(board[0] + ' | ' + board[1] + ' | ' + board[2])
    print(board[3] + ' | ' + board[4] + ' | ' + board[5])
    print(board[6] + ' | ' + board[7] + ' | ' + board[8])
displayBoard()

def inputLetters():
    global currentLetter
    request = input(currentLetter + "'s turn || Choose a position from 1 - 9: ")
    print()
    while int(request) not in range(1,len(board)+1):
        request = input('Invalid input. Please choose a position from 1 - 9: ')
    if board[int(request)-1] == 'X' or board[int(request)-1] == 'Y':
        currentLetter = currentLetter
    else:
        board[int(request)-1] = currentLetter
        checkForWin()
        changingLetters()
    
def changingLetters():
    global currentLetter
    if currentLetter == 'X':
        currentLetter = 'O'
    else:
        currentLetter = 'X'
        
def checkForWin():
    global win
    if board[0] == board[1] == board[2] == 'X' or board[0] == board[1] == board[2] == 'Y':
        print('The ' + currentLetter + ' player has won!')
        win = True
        if board[3] == board[4] == board[5] == 'X' or board[3] == board[4] == board[5] == 'Y':
            print('The ' + currentLetter + ' player has won!')
            win = True
            if board[6] == board[7] == board[8] == 'X'or board[6] == board[7] == board[8] == 'Y':
                print('The ' + currentLetter + ' player has won!')
                win = True
                if board[0] == board[3] == board[6] == 'X'or board[0] == board[3] == board[6] == 'Y':
                    print('The ' + currentLetter + ' player has won!')
                    win = True
                    if board[1] == board[4] == board[7] == 'X' or board[1] == board[4] == board[7] == 'Y':
                        print('The ' + currentLetter + ' player has won!')
                        win = True
                        if board[2] == board[5] == board[8] == 'X' or board[2] == board[5] == board[8] == 'Y':
                            print('The ' + currentLetter + ' player has won!')
                            win = True
                            if board[2] == board[4] == board[6] == 'X' or board[2] == board[4] == board[6] == 'Y':
                                print('The ' + currentLetter + ' player has won!')
                                win = True
                                if board[0] == board[4] == board[8] == 'X' or board[0] == board[4] == board[8] == 'Y':
                                    print('The ' + currentLetter + ' player has won!')
                                    win = True
    elif '-' not in board:
        print('Tie')

while True:
    inputLetters()
    displayBoard()
    if win:
        break
\$\endgroup\$
3
  • 2
    \$\begingroup\$ please put whole code in the question. \$\endgroup\$
    – hjpotter92
    Sep 22 '20 at 15:28
  • 3
    \$\begingroup\$ This program isn't correct. Have you tried running it yourself? Requirements for posting are that the program works as intended. \$\endgroup\$
    – user985366
    Sep 22 '20 at 16:07
  • 3
    \$\begingroup\$ The code you have provided does not work as expected. Which is off-topic \$\endgroup\$
    – Parekh
    Nov 4 '20 at 4:50
2
\$\begingroup\$

This is a month old, hope this review is still helpful!

  • learn how to pass things into a function, and return things from a function.
  • you use global state (all the variables like the board, whose turn it is, and whether they won). that's okay while you're learning, but you should avoid this once you know how.
  • each function should clearly do one thing, so someone reading it can understand what the function call does without reading the function. this will help you too, as your programs get bigger and bigger.
    • 'displayBoard' is perfect--it displays the board.
    • 'inputLetters' does a lot of things--it should just return which letter the player picked instead.
    • 'checkforWin' is great, it does exactly one thing. it should return True or False, instead of setting the global 'win'.
  • checkForWin is long and complicated (also, copy-pasted wrong, probably). it's very repetitive. in programming, this is considered a Bad Sign--you should figure out a way to write it to be less repetitive. That way it will be easier to look at and understand. i'm going to leave this one as a challenge to you
  • all the code not in a function, should be together at the end, so it's easy to find.
\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review. This is a very nice answer, but in general please don't answer questions that have a comment that indicates the question is off-topic. \$\endgroup\$
    – pacmaninbw
    Nov 4 '20 at 14:36
  • \$\begingroup\$ The question was edited to be on-topic after that comment (originally this was 5 lines of code with the rest missing), which I checked. \$\endgroup\$ Nov 5 '20 at 9:09

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