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

`

\$\endgroup\$
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.

\$\endgroup\$
  • 1
    \$\begingroup\$ @RolandIllig You are absolutely correct. I've updated my answer. \$\endgroup\$ – rickdenhaan May 8 '17 at 20:47

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.