4
\$\begingroup\$

Thank you for reviewing my code. I know C++ and I am learning python. As a beginner project I wrote a simple tic-tac-toe game using python3. It would be helpful if you could:

  1. Tell whether there is anything fundamentally wrong, my coding style, variable name etc.
  2. Tell whether I am following python standards.
  3. Better way to design the algorithm
  4. Further Improvements.

Code: `

#!/usr/bin/env python
import random
import os
class Tic_Tac_Toe(object):
 #used to print the status of the game
 def PrintTable(self, position, turn):
 print("\n")
 print("\n")
 #position tells whose turn currently is
 position -= 1
 if turn == 1:
 self.table[position] = "X"
 elif turn == -1:
 self.table[position] = "O"
 #formatting the table
 for i in range(0,(self.no_of_rows)):
 print('\t\t\t', end='')
 for j in range(0,(self.no_of_rows)):
 if self.table[(i*self.no_of_rows)+j] == "X" or \
 self.table[(i*self.no_of_rows)+j] == "O":
 print(" | ", end='')
 print(self.table[(i*(self.no_of_rows))+j], end = '')
 else: 
 print(" | ", end='')
 print(self.table[(i*(self.no_of_rows))+j], end = '')
 print(" |\n")
 #checking for winner
 self.game_over = self.CheckWinner()
 def Play(self):
 #to avoid problems in formatting eg. 9 requires 1 space but 12 requires 2 spaces
 #table looks hapazard so adding a 0 before numbers from 1 - 9
 self.table = []
 for i in range (0,(self.no_of_rows)*(self.no_of_rows)):
 if i <= 8:
 self.table.append("0" + str(i+1))
 else:
 self.table.append(i+1)
 position = 0
 turn = 0
 self.PrintTable(position, turn)
 #to make player1's symbol as "X" always
 if self.player2_symbol == "X":
 self.player1_symbol, self.player2_symbol = self.player2_symbol, self.player1_symbol
 self.player1_name, self.player2_name = self.player2_name, self.player1_name
 counter = 0
 turn = int(1)
 while(counter < (self.no_of_rows)*(self.no_of_rows)):
 counter += 1
 if turn == 1:
 print("\n\t\t{0} it's your turn. Enter any position (1-{1}): " .format(self.player1_name, self.no_of_rows * self.no_of_rows), end = '')
 else:
 print("\n\t\t{0} it's your turn. Enter any position (1-{1}): " .format(self.player2_name, self.no_of_rows * self.no_of_rows), end = '')
 position = int(input())
 #checking for correct input
 if (position >= 1 and
 position <= ((self.no_of_rows)*(self.no_of_rows))):
 if(self.table[position-1] != "X" and
 self.table[position-1] != "O"):
 self.PrintTable(position, turn)
 if self.game_over == "X":
 print("\n\n\t\t\tGame Over! {0} wins!!!". format(self.player1_name))
 break
 elif self.game_over == "O":
 print("\n\n\t\t\tGame Over! {0} wins!!!". format(self.player2_name))
 break
 else:
 counter -= 1
 print("\n\t\tPosition you entered is already filled!")
 continue
 else:
 counter -= 1
 print("\n\t\tPlease enter a position in the range (1-{0})!".format(self.no_of_rows * self.no_of_rows))
 continue
 turn *= (-1)
 if self.game_over == "-1":
 print("\n\n\t\t\tMatch ends in a Draw!!")
 def New_Game(self):
 print("033円[1;31m\n\n\t\t\t TIC-TAC-TOE !!\n\t\t\t --- --- ---")
 while(1):
 self.no_of_rows = int(input("\n\t\tEnter number of rows(2 - 9): "))
 if self.no_of_rows > 9 or self.no_of_rows < 2:
 print("\n\t\t Row number is restricted to range [2,9]!")
 else:
 break
 self.player1_name = input("\n\n\t\tEnter the name of Player1: ")
 if self.player1_name == "":
 self.player1_name = "Player1"
 self.player2_name = input("\n\n\t\tEnter the name of Player2: ")
 if self.player2_name == "":
 self.player2_name = "Player2"
 print("\n\n\t\t\t\t Toss-Time!")
 while(1):
 player_toss = input("\n\n\t\t\t {0} it's your call(H or T): " .format(self.player1_name));
 if (player_toss == "H" or
 player_toss == "h" or
 player_toss == "T" or
 player_toss == "t"):
 break
 else:
 print("\n\t\t\tYou have to call either H or T")
 if player_toss == "H" or player_toss == "T":
 player_toss = player_toss.lower()
 toss = random.randint(0,1)
 if ((player_toss == 'h' and toss == 0) or (player_toss == 't' and toss == 1)):
 print("\n\n\t\t\t{0} takes X and plays first." .format(self.player1_name))
 self.player1_symbol = "X"
 self.player2_symbol = "O"
 else:
 print("\n\n\t\t\t{0} takes X and plays first." .format(self.player2_name))
 self.player2_symbol = "X"
 self.player1_symbol = "O"
 self.Play()
 #to check whether game has ended or not
 def CheckWinner(self):
 is_game_over = True
 #checking primary diagonal
 for i in range (1, self.no_of_rows):
 if self.table[(i*self.no_of_rows)+i] != self.table[((i-1)*self.no_of_rows)+(i-1)]:
 is_game_over = False
 break
 if is_game_over == True:
 return self.table[0]
 is_game_over = True
 #checking secondary diagonal
 for i in range (1, self.no_of_rows):
 if self.table[(i*self.no_of_rows)+((self.no_of_rows-1)-i)] != self.table[((i-1)*(self.no_of_rows))+((self.no_of_rows-1)-(i-1))]:
 is_game_over = False
 break
 if is_game_over == True:
 return self.table[(self.no_of_rows-1)]
 is_game_over = True
 #checking rows
 for i in range (0, self.no_of_rows):
 is_game_over = True
 for j in range (1, self.no_of_rows):
 if self.table[(i*(self.no_of_rows))+j] != self.table[(i*(self.no_of_rows))+j-1]:
 is_game_over = False
 break
 if is_game_over == True:
 return self.table[(i*(self.no_of_rows))]
 #checking columns
 for i in range (0, (self.no_of_rows)):
 is_game_over = True
 for j in range (1, (self.no_of_rows)):
 if self.table[(j*(self.no_of_rows))+i] != self.table[((j-1)*(self.no_of_rows))+i]:
 is_game_over = False
 break
 if is_game_over == True:
 return self.table[(i)]
 #if winner cannot be decided at this stage
 return "-1"
while(1):
 T = Tic_Tac_Toe()
 T.New_Game()
 play_again = input("\n\n\t\t\t Want to play again?(y/n): ")
 if play_again == 'y' or play_again == 'Y':
 os.system("clear")
 continue
 else:
 print("\n\n\t\t\t\t GoodBye :)")
 break

`

asked May 7, 2017 at 12:46
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

By using the hashbang #!/usr/bin/env python, on most systems this will actually still trigger Python 2. For most of what you're doing, that won't make a big difference, except for two things:

  1. Your use of the print() function. To be safe, I'd add a from __future__ import print_function just to be safe.
  2. Your use of the input() function. In Python 2, this worked slightly different. To be safe, I'd try: to overwrite input with raw_input, except: if a NameError occurs. Then just pass.

If you only want to support Python 3, I'd change the hashbang to use python3 instead of plain python.

One major annoyance: when the game starts, you turn the text color to red. You don't reset it when the game ends. So after running the game in my terminal, all text stays red until I close my terminal or manually output an ANSI code to reset the text color...

As far as Python standards are concerned, it's customary to add docblocks in triple-quotes as the first lines in functions where you explain what the point of the function is and preferably describe the arguments and return value. For example:

def PrintTable(self, position, turn):
 """
 Outputs the Tic-Tac-Toe playing field.
 :param position : current player
 :param turn : whose turn it is
 :type position : int
 :type turn : int
 """
 print("\n")

These docblocks are useful in editors that support type hinting for code highlighting and error checking, like PyCharm. The :type lines tell these editors that position is an int, so if you accidentally call position.encode("utf-8") for example, it will warn you that you can't do that. Also, these docblocks are printed when someone wants to use your code for something else, and needs info using the help() function:

# ttt.py is the file containing your code
import ttt
T = ttt.Tic_Tac_Toe()
help(T.PrintTable)
# will output the docblock for the PrintTable method

Code style and variable names are generally OK for my taste, although I really don't like naming variables i and j in iterations. I'd personally rename them to row and column:

#formatting the table
for row in range(0,(self.no_of_rows)):
 print('\t\t\t', end='')
 for column in range(0,(self.no_of_rows)):
 if self.table[(row*self.no_of_rows)+column] == "X" or \

Also, you're using a lot of brackets where they're not really needed:

for row in range(0,(self.no_of_rows)):
# is identical to
for row in range(0,self.no_of_rows):
print(self.table[(i*(self.no_of_rows))+j], end = '')
# is identical to
print(self.table[(i*self.no_of_rows)+j], end = '')
# and technically to
print(self.table[i*self.no_of_rows+j], end = '')
# because of order of priority in math, but I'd personally keep the 
# brackets around the multiplication for clarity
while(counter < (self.no_of_rows)*(self.no_of_rows)):
# is identical to
while counter < self.no_of_rows*self.no_of_rows:
# because a while does not need brackets
if (position >= 1 and position <= ((self.no_of_rows)*(self.no_of_rows))):
# is identical to
if position >= 1 and position <= self.no_of_rows*self.no_of_rows:
# because an if-statement also does not need brackets
# this if-statement can also be shortened to
if 1 < position <= self.no_of_rows*self.no_of_rows:
# although that is admittedly less transparent

As for the while(1): loops, it's more common in Python to write those as while True:.

It is customary in Python classes to have an __init__(self) method that sets all internal properties to their initial states. For your Tic_Tac_Toe class, that would be:

class Tic_Tac_Toe(object):
 def __init__(self):
 self.table = []
 self.no_of_rows = 0
 self.game_over = None
 self.player1_symbol = "X"
 self.player1_name = "Player1"
 self.player2_symbol = "O"
 self.player2_name = "Player2"

Finally, because people may want to use your file as a module and extend it with their own code, it's usually not appreciated to have a file automatically do stuff when it's imported. That while(1) loop at the bottom that automatically starts a new game, should be wrapped with an if statement to check if the file with the game was actually called directly:

if __name__ == "__main__":
 while(1):
 T = Tic_Tac_Toe()

That way, I can run ttt.py and play a game, or I can run my_ttt.py which has my own code with import ttt that does not automatically play a game but maybe runs some automated tests or extends your game with some of my own logic.

answered May 7, 2017 at 20:02
\$\endgroup\$
1
  • 1
    \$\begingroup\$ @RolandIllig You are absolutely correct. I've updated my answer. \$\endgroup\$ Commented May 8, 2017 at 20:47

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.