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:
- Tell whether there is anything fundamentally wrong, my coding style, variable name etc.
- Tell whether I am following python standards.
- Better way to design the algorithm
- 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
`
1 Answer 1
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:
- Your use of the
print()
function. To be safe, I'd add afrom __future__ import print_function
just to be safe. - Your use of the
input()
function. In Python 2, this worked slightly different. To be safe, I'dtry:
to overwriteinput
withraw_input
,except:
if aNameError
occurs. Then justpass
.
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.
-
1\$\begingroup\$ @RolandIllig You are absolutely correct. I've updated my answer. \$\endgroup\$rickdenhaan– rickdenhaan2017年05月08日 20:47:25 +00:00Commented May 8, 2017 at 20:47