3
\$\begingroup\$

I am new to using Classes and Objects, and as such thought i would edit a previous Tic Tac Toe game i created, this time using Classes. I found it easier using entirely public (Class?) variables (ie, ClassName.Variable). This way any variable can be accessed from anywhere, making utilising said variables much easier across the code. Lists are then used to store things like the board, the game types, and in other cases besides this one, whose go it would be next (ie, when you introduce three players) etc. Lists are beautiful things.

I am aware i have put self in some initialisers, but not actually used it. i am also aware some Public variables are named Class1.Variable when they are only ever used in Class2, as i set them to whichever class the variable would belong to.

Oh, and i always use Capitals for Class/Function/Variable names. i know its frowned upon but i personally find it much more organised and neat :)

I would like to ask :

How can i use self and protected/private variables to best suit the game of Tic Tac Toe and avoid all-Public variables?

Is it a Crime to use entirely Public variables?

Are there any other issues or problems with my coding style, and how can i correct that?

Original Code

from random import * 
from time import *
#Noughts and Crosses
#Only works in Python 3

def PrintBoard(Places): #Prints the Board, mirrors a laptop's keypad
    print('',Places[6],'|',Places[7],'|',Places[8],'')
    print('---|---|---')
    print('',Places[3],'|',Places[4],'|',Places[5],'')
    print('---|---|---')
    print('',Places[0],'|',Places[1],'|',Places[2],'')
    print()#empty line at the bottom to make room for further prompts


def VerifyPosition(Places, Type):#Verifys and asks for your selection
    Valid = False
    print(Type+"'s Go...")#O's Go... / X's Go...
    while not Valid:
        Position = input("Please type where you wish to place: ")
        while Position.isdigit() == False or int(Position) > 9 or int(Position) <= 0:#Ensures the player does not type an incorrect location to place in
            Position = input("Please type a valid integer between 1 and 9: ")
        Position = int(Position)
        if Places[Position-1] == 'X' or Places[Position-1] == 'O':#9 on the keypad would refer to index 8, as starts from 0 but keypad starts at 1
            Valid = False
            print("That is an invalid square, please try again ")
        else:
            Valid = True
    Places[Position-1] = Type#lists start at 0, inputs start at 1
    #the board is layed out identical to a Computer's Key-Pad, hence starting at 1 and not 0, requiring -1 to the positional index


def PlayRoundSingle(Places, Type, GameType):
    GameWon = False
    while not GameWon:#loops until someone wins
        if GameType in ["S","M"]:
            Move(Places, Type)
            GameWon,WinningType = CheckWin(Places, Type)
            Type = ChangeType(Type)
        if GameType != "M" and GameWon == False:# If Single Player and the Game has not been Won...
            print(Type+"'s Go...")#Computers Go!
            sleep(1)
            Places = GetComputersChoice(Places, Type)
            GameWon,WinningType = CheckWin(Places, Type)
            Type = ChangeType(Type)
    return WinningType, GameWon

def GetComputersChoice(Places, Type):
    EmptySpaces = []
    for i in range(0,9):#goes through all spaces on the board...
        if Places[i] == ' ': #Finds empty spaces...
            EmptySpaces.append(i) # Adds them to a list for future reference
    Places = ComputerMove(Places, EmptySpaces, Type) 
    PrintBoard(Places) # Prints the Board
    return Places

def ComputerMove(Places,EmptySpaces, Type):
    Change = False
    OriginalType = Type # prevents placing down the opponent's piece when checking for a BLOCK
    for i in range(0,2):#Checks each space twice, FIRST to see if there is a winning move, THEN to check if they can block the opponent
        for i in range(0,len(EmptySpaces)):#check the space to see FIRST if two friendly pieces are in line for a win, THEN check if you can block an opponent, IF NEITHER, radomise
            if not Change: # If no change has happened (prevents re-checking after a space has been found
                Position, Change = CheckComputerWin(Places, EmptySpaces[i], Type) # checks to see whether the computer can cause a three-in-a-row... or prevent one
            if Change:    # If there is a change...
                Places[Position] = OriginalType # Fills in the space
                return Places # prevents unneccesarily checking more spaces
        Type = ChangeType(Type) #switches the type, to see if any draws are available AFTER checking for wins
    Places[choice(EmptySpaces)] = OriginalType # If no places were found to have a effect, randomize the location...
    return Places

def CheckComputerWin(Places, SpaceToCheck, Type):
    if (SpaceToCheck in [6,3,0] and Places[SpaceToCheck + 1] == Type and Places[SpaceToCheck + 2] == Type) or (#Left Side Checks     |
        SpaceToCheck in [7,4,1] and Places[SpaceToCheck + 1] == Type and Places[SpaceToCheck - 1] == Type) or (#Central Column check | Horizontal Checks
        SpaceToCheck in [8,5,2] and Places[SpaceToCheck - 1] == Type and Places[SpaceToCheck - 2] == Type) or (#Right Side Check     |
        SpaceToCheck in [6,7,8] and Places[SpaceToCheck - 3] == Type and Places[SpaceToCheck - 6] == Type) or (#Top Row Check     |
        SpaceToCheck in [5,4,3] and Places[SpaceToCheck - 3] == Type and Places[SpaceToCheck + 3] == Type) or (#Middle Row Check  | Vertical Checks
        SpaceToCheck in [2,1,0] and Places[SpaceToCheck + 3] == Type and Places[SpaceToCheck + 6] == Type) or (#Bottom Row Check  |
        SpaceToCheck == 0 and Places[SpaceToCheck + 4] == Type and Places[SpaceToCheck + 8] == Type) or ( #Bottom Left  |
        SpaceToCheck == 2 and Places[SpaceToCheck + 2] == Type and Places[SpaceToCheck + 4] == Type) or ( #Bottom Right | Diagonal
        SpaceToCheck == 6 and Places[SpaceToCheck - 2] == Type and Places[SpaceToCheck - 4] == Type) or ( #Top Left     | Checks
        SpaceToCheck == 8 and Places[SpaceToCheck - 4] == Type and Places[SpaceToCheck - 8] == Type) or ( #Top Right    |
        SpaceToCheck == 4 and ((Places[SpaceToCheck + 2] == Type and Places[SpaceToCheck - 2] == Type) or ( # Centre Piece, | Top-Left to Bottom-Right  | Diagonal
            Places[SpaceToCheck + 4] == Type and Places[SpaceToCheck - 4] == Type))):                       # Centre Piece, | Top-Right to Bottom-Left  | Checks
        return SpaceToCheck, True # Yes, The position Places[SpaceToCheck] would not win or block an opponent
    return 0, False    # No, The position Places[SpaceToCheck] would not win or block an opponent



def ChangeType(Type):#Switches the players turns after their go
    if Type == 'X':
        Type = 'O'
    else:
        Type = 'X'
    return Type


def Move(Places, Type):
    VerifyPosition(Places, Type)#asks for your square,verifies, then moves
    PrintBoard(Places)#shows the board


def CheckWin(Places,Type):
    GameWon = False
    if ((Places[0]== Type) and (Places[1] == Type) and (Places[2] == Type)) or (
        (Places[3]== Type) and (Places[4] == Type) and (Places[5] == Type)) or (
        (Places[6]== Type) and (Places[7] == Type) and (Places[8] == Type)) or (
        #Row Check^^^^^^^
        (Places[0]== Type) and (Places[3] == Type) and (Places[6] == Type)) or (
        (Places[1]== Type) and (Places[4] == Type) and (Places[7] == Type)) or (
        (Places[2]== Type) and (Places[5] == Type) and (Places[8] == Type)) or (
        #Column Check^^^^
        (Places[0]== Type) and (Places[4] == Type) and (Places[8] == Type)) or (
        (Places[2]== Type) and (Places[4] == Type) and (Places[6] == Type)):
        #Diagonal Check^^
      #  Type = 'X'
        GameWon = True
    if GameWon != True:#if no one wins (potentially on the last go...)
        DrawCheck = 0
        for i in range(0,9):#checks the number of spaces left in the board out of 9 squares, 0-8
            if Places[i] == ' ':
                DrawCheck = DrawCheck + 1
        if DrawCheck == 0:#if there are no squares...
            GameWon = 'Draw'

    return GameWon, Type#returns True, False or 'Draw' and Type (who won, NONE if else)


def Start():
    print("Noughts and Crosses")
    print("1 - Play Single Player, against the Computer")
    print("2 - Play Multiplayer with a friend on the same computer")
    print("3 - Play with... oh wait, this isn't right. press 3 to not play at all?")
    Choice = input("Please Enter your choice: ")
    while Choice not in ['1','2','3']:
        Choice = input("Please type 1,2 or 3 as your choice: ")
    if Choice == '1':
        GameType = "S"
    elif Choice == '2':
        GameType = "M"
    else:
        print("\nComputer v Computer it is...\n") 
        GameType = "Computer v Computer"
    if GameType in ["S","M"]:
        Places = ['1','2','3',#for the initial board - to show the player which key relates to which position
                  '4','5','6',
                  '7','8','9']#flipped as Key-Pads are flipped
        print("This is how the Board is layed Out (Key-Pad use recommended)")
        PrintBoard(Places)
    Places = [' ',' ',' ',
              ' ',' ',' ',
              ' ',' ',' ']
    if GameType not in["S","M"]:
        PrintBoard(Places)
        sleep(1)
    Type = choice(['X','O'])#more effective than randint
    WinningType, GameWon = PlayRoundSingle(Places, Type, GameType)
#waiting for game to end...
    if GameWon == True:#if you win...
        print(WinningType, "Wins!")
    else:#if you draw... (will not get to this stage is GameWon is False, as will stay in the loop
        print("Draw!")



#starts here...
PlayAgain = 'yes'
while PlayAgain in ["Yes","yes","y","Y"]:
    Start()
    PlayAgain = input("Do You want to play again? Y/es or N/o: ")
    print()

#quit()

Classes Version

from random import *
from time import *


class Play:
    def __init__(self):#Menu - maybe shouldn't be an initialiser
    #self in brackets but not used, i know, it's icky... but it works
        print("Noughts and Crosses")
        print("1 - Play Single Player, against the Computer")
        print("2 - Play Multiplayer with a friend on the same computer")
        print("3 - Play with... oh wait, this isn't right. press 3 to not play at all?")
        Play.Choice = input("Please Enter your Choice: ")
        while Play.Choice not in ['1','2','3']:
            Play.Choice = input("Please type 1,2 or 3 as your Choice: ")
        if Play.Choice == '1':
            Play.GameType = "S"
        elif Play.Choice == '2':
            Play.GameType = "M"
        else:
            print("\nComputer v Computer it is...\n") 
            Play.GameType = "Computer v Computer" # not actally reffered to directly, so can be anything
        Board()
        Play.Type = choice(['X','O'])#more effective than randint
        Play.Round()

        #waiting for game to end...

        if Play.GameWon == True:#if you win...
            print(Play.WinningType, "Wins!")
        else:#if you draw... (will not get to this stage is GameWon is False, as will stay in the loop
            print("Draw!")
        PlayAgain = input("Do you want to play again? Y/es or N/o: ")
        if PlayAgain in ["Yes", "yes", "y", "Y"]:
            print()
            Play()

    def Round():
        Play.GameWon = False
        while not Play.GameWon:#loops until someone wins
            if Play.GameType in ["S","M"]:
                Play.Move()
                Play.CheckWin()
                Play.ChangeType()
            if Play.GameType != "M" and Play.GameWon == False:# If Single Player and the Game has not been Won...
                print(Play.Type+"'s Go...")#Computers Go!
                sleep(1)
                Computer.GetComputersChoice()
                Play.CheckWin()
                Play.ChangeType()

    def Move():
        Play.VerifyPosition()#asks for your square,verifies, then moves
        Board.PrintBoard()#shows the board

    def ChangeType():
        if Play.Type == 'X':
            Play.Type = 'O'
        else:
            Play.Type = 'X'

    def VerifyPosition():
        Valid = False
        print(Play.Type+"'s Go...")#O's Go... / X's Go...
        while not Valid:
            Position = input("Please type where you wish to place: ")
            while Position.isdigit() == False or int(Position) > 9 or int(Position) <= 0:#Ensures the player does not type an incorrect location to place in
                Position = input("Please type a valid integer between 1 and 9: ")
            Play.Position = int(Position)
            if Board.Places[Play.Position-1] == 'X' or Board.Places[Play.Position-1] == 'O':#9 on the keypad would refer to index 8, as starts from 0 but keypad starts at 1
                Valid = False
                print("That is an invalid square, please try again ")
            else:
                Valid = True
        Board.Places[Play.Position-1] = Play.Type#lists start at 0, inputs start at 1, hence -1
        #the board is layed out identical to a Computer's Key-Pad, hence starting at 1 and not 0, requiring -1 to the positional index

    def CheckWin():
        if ((Board.Places[0]== Play.Type) and (Board.Places[1] == Play.Type) and (Board.Places[2] == Play.Type)) or (
            (Board.Places[3]== Play.Type) and (Board.Places[4] == Play.Type) and (Board.Places[5] == Play.Type)) or (
            (Board.Places[6]== Play.Type) and (Board.Places[7] == Play.Type) and (Board.Places[8] == Play.Type)) or (
            #Row Check^^^^^^^
            (Board.Places[0]== Play.Type) and (Board.Places[3] == Play.Type) and (Board.Places[6] == Play.Type)) or (
            (Board.Places[1]== Play.Type) and (Board.Places[4] == Play.Type) and (Board.Places[7] == Play.Type)) or (
            (Board.Places[2]== Play.Type) and (Board.Places[5] == Play.Type) and (Board.Places[8] == Play.Type)) or (
            #Column Check^^^^
            (Board.Places[0]== Play.Type) and (Board.Places[4] == Play.Type) and (Board.Places[8] == Play.Type)) or (
            (Board.Places[2]== Play.Type) and (Board.Places[4] == Play.Type) and (Board.Places[6] == Play.Type)):
            #Diagonal Check^^
            Play.WinningType = Play.Type
            Play.GameWon = True
        if Play.GameWon != True:#if no one wins (potentially on the last go...)
            DrawCheck = 0
            for i in range(0,9):#checks the number of spaces left in the board out of 9 squares, 0-8
                if Board.Places[i] == ' ':
                    DrawCheck = DrawCheck + 1
            if DrawCheck == 0:#if there are no squares...
                Play.GameWon = 'Draw'

class Computer:
    def GetComputersChoice():
        Computer.FindEmptySpaces()
        Computer.ComputerMove()
        Board.PrintBoard() # Prints the Board

    def FindEmptySpaces():
        Computer.EmptySpaces = []
        for i in range(0,9):#goes through all spaces on the board...
            if Board.Places[i] == ' ': #Finds empty spaces...
                Computer.EmptySpaces.append(i) # Adds them to a list for future reference

    def ComputerMove():
        Computer.Change = False
        OriginalType = Play.Type # prevents placing down the opponent's piece when checking for a BLOCK
        for j in range(0,2):#Checks each space twice, FIRST to see if there is a winning move, THEN to check if they can block the opponent
            for i in range(0,len(Computer.EmptySpaces)):#check the space to see FIRST if two friendly pieces are in line for a win, THEN check if you can block an opponent, IF NEITHER, radomise
                if not Computer.Change: # If no change has happened (prevents re-checking after a space has been found)
                    Computer.CheckComputerWin((Computer.EmptySpaces[i])) # checks to see whether the computer can cause a three-in-a-row... or prevent one
                if Computer.Change:    # If there is a change...
                    Board.Places[Computer.EmptySpaces[i]] = OriginalType # Fills in the space
                    Play.Type = OriginalType
                    return
            Play.ChangeType() #switches the type, to see if any draws are available AFTER checking for wins
        Play.Type = OriginalType
        Board.Places[choice(Computer.EmptySpaces)] = OriginalType # If no places were found to have a effect, randomize the location...

    def CheckComputerWin(SpaceToCheck):
        if (SpaceToCheck in [6,3,0] and Board.Places[SpaceToCheck + 1] == Play.Type and Board.Places[SpaceToCheck + 2] == Play.Type) or (#Left Side Checks     |
            SpaceToCheck in [7,4,1] and Board.Places[SpaceToCheck + 1] == Play.Type and Board.Places[SpaceToCheck - 1] == Play.Type) or (#Central Column check | Horizontal Checks
            SpaceToCheck in [8,5,2] and Board.Places[SpaceToCheck - 1] == Play.Type and Board.Places[SpaceToCheck - 2] == Play.Type) or (#Right Side Check     |
            SpaceToCheck in [6,7,8] and Board.Places[SpaceToCheck - 3] == Play.Type and Board.Places[SpaceToCheck - 6] == Play.Type) or (#Top Row Check     |
            SpaceToCheck in [5,4,3] and Board.Places[SpaceToCheck - 3] == Play.Type and Board.Places[SpaceToCheck + 3] == Play.Type) or (#Middle Row Check  | Vertical Checks
            SpaceToCheck in [2,1,0] and Board.Places[SpaceToCheck + 3] == Play.Type and Board.Places[SpaceToCheck + 6] == Play.Type) or (#Bottom Row Check  |
            SpaceToCheck == 0 and Board.Places[SpaceToCheck + 4] == Play.Type and Board.Places[SpaceToCheck + 8] == Play.Type) or ( #Bottom Left  |
            SpaceToCheck == 2 and Board.Places[SpaceToCheck + 2] == Play.Type and Board.Places[SpaceToCheck + 4] == Play.Type) or ( #Bottom Right | Diagonal
            SpaceToCheck == 6 and Board.Places[SpaceToCheck - 2] == Play.Type and Board.Places[SpaceToCheck - 4] == Play.Type) or ( #Top Left     | Checks
            SpaceToCheck == 8 and Board.Places[SpaceToCheck - 4] == Play.Type and Board.Places[SpaceToCheck - 8] == Play.Type) or ( #Top Right    |
            SpaceToCheck == 4 and ((Board.Places[SpaceToCheck + 2] == Play.Type and Board.Places[SpaceToCheck - 2] == Play.Type) or ( # Centre Piece, | Top-Left to Bottom-Right  | Diagonal
                Board.Places[SpaceToCheck + 4] == Play.Type and Board.Places[SpaceToCheck - 4] == Play.Type))):                     # Centre Piece, | Top-Right to Bottom-Left  | Checks
            Computer.Change = True
class Board:
    def __init__(self):
        if Play.GameType in ["S", "M"]:
            Board.Places = ['1','2','3',#for the initial board - to show the player which key relates to which position
                            '4','5','6',
                            '7','8','9']#flipped as Key-Pads are flipped
            print("This is  how the Board is layed Out (Key-Pad use recommended)")
            Board.PrintBoard()
        Board.Places = [' ',' ',' ',
                        ' ',' ',' ',
                        ' ',' ',' ']
        if Play.GameType not in ["S","M"]:
            Board.PrintBoard()
            sleep(1)

    def PrintBoard(): #Prints the Board, mirrors a laptop's keypad
        print('',Board.Places[6],'|',Board.Places[7],'|',Board.Places[8],'')
        print('---|---|---')
        print('',Board.Places[3],'|',Board.Places[4],'|',Board.Places[5],'')
        print('---|---|---')
        print('',Board.Places[0],'|',Board.Places[1],'|',Board.Places[2],'')
        print()#empty line at the bottom to make room for further prompts


Play()
\$\endgroup\$
  • 2
    \$\begingroup\$ FYI, there is a Python style guide. It's not just the naming conventions you aren't compliant with; you're entitled to your opinions but when you're sharing that code expect to keep getting told you're wrong. \$\endgroup\$ – jonrsharpe Mar 22 '17 at 12:31
  • \$\begingroup\$ Thank you for that link. I am aware of the naming issues, but have learned that it is not absolutely necessary, albeit highly recommended. If that (or any other issues) arise, i'm open to learn what's wrong and why, so (constructive) criticism is okay \$\endgroup\$ – David Mar 22 '17 at 13:12
  • \$\begingroup\$ stackoverflow.com/q/1641219/6655092 \$\endgroup\$ – JakeD Mar 22 '17 at 13:33
  • \$\begingroup\$ There are a lot of beginners python tic-tac-toe in code review but for some reason, nobody realizes that "is it a draw?" actually simply means "is this the 10th turn?", I wonder why. \$\endgroup\$ – ChatterOne Mar 22 '17 at 14:09
4
\$\begingroup\$

I'm not going to go into line by line detail, because there's little there I would salvage and I don't think just writing you an actual OOP version counts as code review. Instead, I want to talk about your approach.

I am new to using Classes and Objects, and as such thought i would edit a previous Tic Tac Toe game i created, this time using Classes.

This is a good idea. Rewriting existing code in new paradigms can give you a feel for their strengths and weaknesses. Sometimes you'll feel a tension where a particular paradigm doesn't fit a particular task, and that provides useful information.

I found it easier using entirely public (Class?) variables (ie, ClassName.Variable). This way any variable can be accessed from anywhere, making utilising said variables much easier across the code.

That is a terrible reason to do something. The use of global state in general is strongly discouraged because it makes your programs harder to understand and test. In particular it seems ludicrous to try an "OOP" approach that only uses class objects, never instances. You should be able to instantiate and test a Board completely independent of the game and the players, for example; if it doesn't have to know or care about those things and can focus purely on spaces with or without values in it becomes much simpler.

Lists are then used to store things like the board, the game types, and in other cases besides this one, whose go it would be next (ie, when you introduce three players) etc. Lists are beautiful things.

I would probably use nested lists for the board. A flat list is fine, but (row, col) indexing makes the tasks involved in tic tac toe a bit more straightforward.

However there is a clear problem in the way you reverse the board for printing; if you used the order it was displayed in to start with, the code would be simpler and you wouldn't have to keep transforming it in your head (another tension you should have listened to, see below).

Also be aware of the other data structures Python offers; the dictionary in particular is everywhere.

I am aware i have put self in some initialisers, but not actually used it.

Why? I mentioned feeling tension above. You clearly felt it here but apparently decided that adding comments that this was probably a bad idea was the right thing to do. Why didn't you instead try to find a good idea? The tension was telling you something; you aren't actually writing classes here. This isn't OOP.

i am also aware some Public variables are named Class1.Variable when they are only ever used in Class2, as i set them to whichever class the variable would belong to.

I'm not sure quite what you mean here, but I suspect you need to read up on "encapsulation" and "separation of concerns".

Oh, and i always use Capitals for Class/Function/Variable names. i know its frowned upon but i personally find it much more organised and neat :)

That's fine if nobody else ever reads your code. But that's rarely the case, and specifically not here where you're asking other people to review it. As a more direct benefit, following the conventions in your own code makes it easier for you to read and understand third party code that also follows them.

How can i use self and protected/private variables to best suit the game of Tic Tac Toe and avoid all-Public variables?

This is more a rewrite than a code review, I've provided some starting points below. But there are loads of resources on OOP in general and in Python specifically out there, do some research and use them.

Is it a Crime to use entirely Public variables?

Literally, no. But also Python is more permissive here than other OOP languages; "we're all consenting adults". Your problem here isn't that they're public, it's that they're all class attributes. You use classes as singleton instances.

Are there any other issues or problems with my coding style, and how can i correct that?.

Read more of other people's code; see the standard library, look at similar stuff on e.g. GitHub. Read other code reviews here, so you can avoid the mistakes others have already had corrected. There are quite a few purely on tic tac toe in Python: https://codereview.stackexchange.com/search?q=%5Bpython%5D+tic+tac+toe+is%3Aq

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