4
\$\begingroup\$

I have spent hours on this code. I am a beginner and open to advice and improvements. I've put # where I feel insecure about the code.

It lets you pick your marker, checks winner, checks draw, ask player if want to play again and checks if the square is occupied.

#TicTacToe
def drawBoard(board):
      print('   |   |')
      print(' ' + board[7] + ' | ' + board[8] + ' | ' + board[9])
      print('   |   |')
      print('-----------')
      print('   |   |')
      print(' ' + board[4] + ' | ' + board[5] + ' | ' + board[6])
      print('   |   |')
      print('-----------')
      print('   |   |')
      print(' ' + board[1] + ' | ' + board[2] + ' | ' + board[3])
      print('   |   |')

game_board = ['#', ' ', ' ',' ', ' ',' ', ' ',' ', ' ',' ']


player1=''
player2=''


def setplayersmove(playermarker):
    print("Select your marker: ")
    playermarker = input(">  ")
    return playermarker



def makemove(someone):
    print("Enter a number 1-9 for your move: ")
    movenum = int(input('> '))
    if movenum < 1 or movenum > 9:
        print("Number between 1 and 9 only.")
        makemove(someone)
    else:#makes sure the square is not occupied by ANYONE
        if game_board[movenum] != player1 and game_board[movenum] != player2:
            game_board[movenum] = someone
        else:
            print("Move already occupied. Try again: ")
            makemove(someone)

def tryagain():
    print("Try again?: Y/N")
    ans = input('> ').lower()
    if ans == 'y':
        gamestart()
    else:
        exit(0)

def checkwinner():
    if game_board[1] == game_board[2] == game_board[3] and game_board[1] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[4] == game_board[5] == game_board[6] and game_board[4] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[7] == game_board[8] == game_board[9] and game_board[7] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[1] == game_board[4] == game_board[7] and game_board[1] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[1] == game_board[5] == game_board[9] and game_board[1] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[2] == game_board[5] == game_board[8] and game_board[2] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[1] == game_board[2] == game_board[3] and game_board[1] != ' ':
        print("We have a winner!")
        tryagain()
    elif game_board[3] == game_board[5] == game_board[7] and game_board[3] != ' ':
        print("We have a winner!")
        tryagain()

def checkdraw():#checking draw. Advice welcome here...
    count = 0
    for item in game_board:
        if item != ' ':
            count += 1
    if count == 10:
        print("---Game Drawn---")
        drawBoard(game_board)
        tryagain()


def gamestart():

    global player1#ways to avoid using global?
    global player2
    global game_board



    game_board = ['#', ' ', ' ',' ', ' ',' ', ' ',' ', ' ',' ']
    drawBoard(game_board)
    print("---Player I--- ")
    player1 = setplayersmove(player1)
    print("---Player II--- ")
    player2 = setplayersmove(player2)


    gameon1 = True

    while gameon1:
        print("---Player I's Turn---")
        makemove(player1)
        drawBoard(game_board)
        gameon2 = True
        checkwinner()
        checkdraw()

        gameon1 = False

        while gameon2:
            print("---Player II's Turn---")
            makemove(player2)
            drawBoard(game_board)
            gameon1 = True
            checkwinner()
            checkdraw()
            gameon2 = False



gamestart()
\$\endgroup\$
3
\$\begingroup\$

You should code to make sure that user input doesn't break any assumptions you are making about the game. For example, I like the gimmick that the players can pick a symbol. However, you should make sure they pick a sensible one. What happens if they pick the same? What about if they pick space?

I would suggest that in the code you use fixed indicators, and replace them when you display them. If not, do some input validation to block weird choices.

As to your question about the draw detection, counting is standard. You could keep a variable to hold the count, and update it each time someone plays. That avoids most of the work recalculating each time.

Your technique for swapping between the two players works and is quite creative. However, I would strongly recommend that you have one outside loop to keep the game going and two distinct inner loops to get player input. This makes it clearer that the behaviour of the players is symmetrical.

Building on that symmetry, you could then have just one input loop and keep a variable recording whose turn it is.

| improve this answer | |
\$\endgroup\$
2
\$\begingroup\$

You are making a common serious mistake that many Python beginners make: misusing functions as if they were goto labels.

Your functions should be more like setplayersmove(), which is the only function here that returns a meaningful value. For example, instead of your checkwinner(), you should have an is_won() function that returns True if the last player won:

def is_won():
    """Return True if the last player won."""
    return (
        game_board[1] != ' ' and game_board[1] == game_board[2] == game_board[3] or
        game_board[4] != ' ' and game_board[4] == game_board[5] == game_board[6] or
        …
        game_board[3] != ' ' and game_board[3] == game_board[5] == game_board[7]
    )

Observe that there is less code repetition, and that this function does exactly one thing. It doesn't decide what to do next — that's the caller's job.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks. Code looks much manageable. Is there a way to return a text string and boolean in is_won() function? \$\endgroup\$ – Obey Existing May 19 '18 at 4:07
  • \$\begingroup\$ No, I don't advising returning both a boolean and a text string. Just let the caller decide what to do. \$\endgroup\$ – 200_success May 19 '18 at 4:09
  • \$\begingroup\$ If I were to use return boolean instead of goto, I feel like I m very limited in terms of what I can do if it were to return True. For example I want the system to print out a statement if a player has won. \$\endgroup\$ – Obey Existing May 19 '18 at 4:13
  • \$\begingroup\$ Like I said, do it in the caller. if is_won(): print('We have a winner!') \$\endgroup\$ – 200_success May 19 '18 at 4:16

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.