3
\$\begingroup\$

This is my first program in Python that I have coded and I am relatively happy with how it turned out. I would like to know how I could make my code shorter and more efficient.

def one_round(player):
    #Distinguish player 1 from player 2
    if player == "p1":
       p = "Player 1"
    elif player == "p2":
         p = "Player 2"

    pin = raw_input("%s move (a/b/c for row | 1/2/3 for column):" % (p)) #receive input for player from console
    #separating row from column
    row = pin[0].lower() 
    col = int(pin[1]) - 1

    if row == "a": #changing the row letter into a digit for the index of board
       row = 0
    elif row =="b":
         row = 1
    elif row == "c":
         row = 2

    while (row > 2 or col > 2) or (board[row][col] == 'X' or board[row][col] == 'O'):
          if row > 2 or col > 2:
             print "Oops, these co-ordinates are off of the board, please try again"

          elif board[row][col] == 'X' or board[row][col] == 'O': 
               print "Oops this spot has been taken, please pick another"

          pin = raw_input("%s move (a/b/c for row | 1/2/3 for column):" % (p)) #receive input for player from console
          #separating row from column
          row = pin[0].lower() 
          col = int(pin[1]) - 1
          if row == "a": #changing the row letter into a digit for the index of board
             row = 0
          elif row =="b":
               row = 1
          elif row == "c":
               row = 2


    if player == "p1": #Determining whether to use X or O
       x = 'X'
    elif player == "p2":
         x = 'O'

    del board[row][col] #Swapping out the dashes for a symbol
    board[row].insert(col, x)

    print str(" ".join(board[0])) + '\n' + str(" ".join(board[1])) + '\n' + str(" ".join(board[2])) #Printing board


    if board[0] == [x, x, x] or board[1] == [x, x, x] or board[2] == [x, x, x] or (board[0][0] == x and board[1][0] == x and board[2][0] == x) or (board[0][1] == x and board[1][1] == x and board[2][1] == x) or (board[0][2] == x and board[1][2] == x and board[2][2] == x) or (board[0][0] == x and board[1][1] == x and board[2][2] == x) or (board[0][2] == x and board[1][1] == x and board[2][0] == x): #Checking for winner
          print '%s wins!' % (p) + '\n' + 'Game Over!' 
          return True
    elif board[0][0] != '-' and board[0][1] != '-' and board[0][2] != '-' and board[1][0] != '-' and board[1][1] != '-' and board[1][2] != '-' and board[2][0] != '-' and board[2][1] != '-' and board[2][2] != '-': #Checking for draw
          print 'Draw!' + '\n' + 'Game Over!'
          return True
    else:
          return False
#---------------------------------------------------------------------------------------------------------------------
"""Tic tac toe game. (Noughts and crosses)"""
print "Welcome to Tic Tac Toe!" + "\n" + " Find a friend and start the game by typing coordinates for a 3x3 grid labelled with the letters a,b,c from top to bottom along the vertical axis and with 1,2,3 from left to right along the horizontal axis." + "\n" + "Have Fun!"
#initialising board and players
board = [["-", "-", "-"], ["-", "-", "-"], ["-", "-", "-"]]
p1 = False
p2 = False
row = 0
col = 0
while (p1 == False and p2 == False): #running game
      p1 = one_round("p1")
      if p1 == True:
         break
      p2 = one_round("p2")
\$\endgroup\$
1
\$\begingroup\$

Welcome to Python xD! You did a great job on it ^^

Here are my suggestions

Don‘t code everything all in one function

there are two parts in your one_round

  • place new piece on board
  • check if there is a winner or game over

so it is better to divide to 2 functions, or more, it is best that one function just do one thing

Clean up lots if else in it

For example:

if row == "a": 
      row = 0
elif row =="b":
      row = 1
elif row == "c":
      row = 2

You can use dictionary instead, board_row = {"a" : 0, "b": 1, "c": 2} or code like row = ord(row) - ord("a")

if player == "p1":
    p = "Player 1"
elif player == "p2":
    p = "Player 2"

AND

if player == "p1":
    x = 'X'
elif player == "p2":
    x = 'O'

Can be replaced by

player_info = {"p1": ["Player 1", "X"], "p2": ["Player 2", "O"]}
...
p, x = player_info[player]

Check win

You did this in a quit straight forward code, so it is a bit long, also in python it is best to just with 79 chars in one line.

  • You can list all position of lines and use a for loop to check if there is a winner(if the current player is the winner)

positions = (((0,0),(0,1),(0,2)), ((1,0),(1,1),(1,2)), ...)

for p in positions:
    if len(set([board[x][y] for x, y in p])) == 1:
        ...
        # then we have a winner, and the winner can only be the current player
  • Or just check the lines around current piece position

directions = (((0,1),(0,-1)), ((1,0),(-1,0)), ((1,1),(1,-1)), ((-1,1),(-1,-1)))

for d in directions:
    for x, y in d:
        line = set()
        current_row, current_col = row, col
        while 0 <= current_row < 3 and 0 <= current_col < 3:
            line.add(board[current_row][current_col])
            current_row += x
            current_col += y
        if len(line) == 1:
            # Win
  • check if it is a draw just need if all('-' not in b for b in board)

Other

del board[row][col] 
board[row].insert(col, x)

You just need do board[row][col] = x

\$\endgroup\$

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.