12
\$\begingroup\$

I built a variant of connect4 in Python. The game runs just in the command line.

It is on a 7×7 board and the players take turns putting tokens in a column (the token falls down) OR turn the board by 90° to the left or right and the tokens fall down again, creating a different board setting, but putting in no token this turn.

I would like to have some feedback on the implementation of the game logic and my Python coding style:

from string import ascii_uppercase


class TFBoard():
    def __init__(self):
        self.width = 7
        self.height = 7
        self.columns = range(1, self.height + 1)
        self.rows = ascii_uppercase[:self.width]
        self.matrix = [[0 for h in range(0, self.height)]
                       for w in range(0, self.width)]
        self.endGame = False
        self.winner = 0
        self.playerTurn = 1
        self.turnNumber = 1

    def flatMatrix(self):
        return [value for array in self.matrix for value in array]

    def drawBoard(self):
        col = ["\033[0m", "\033[91m", "\033[31m", "\033[97m", "\033[92m"]

        def drawInLoops(i, j):
            if i == self.height:
                if j == 0:
                    # bottom left corner
                    print("   ", end=f"")
                else:
                    # bottom letter row
                    print(f"{col[1]}{self.columns[j-1]}{col[0]}", end=f"  ")
            else:
                if j == 0:
                    # left number column
                    print(f"{col[1]}{self.rows[-i-1]}{col[0]}", end=" ")
                else:
                    # squares
                    # drawn matrix is 1 higher and wider than self.matrix
                    print(f"{col[2]}[{col[0]}", end="")
                    piece = self.matrix[j - 1][self.height - i - 1]
                    if piece == 1:
                        piece = f"{col[3]}X{col[0]}"
                    elif piece == 2:
                        piece = f"{col[4]}O{col[0]}"
                    elif piece == 0:
                        piece = " "
                    print(f"{piece}", end="")
                    print(f"{col[2]}]{col[0]}", end="")
        for i in range(0, self.height + 1):
            for j in range(0, self.width + 1):
                drawInLoops(i, j)
            print("")
        print("")  # new line after board for better looks

    def putToken(self, player, column):
        # find first nonzero entrie in column from top
        notification = f"New Player {player} token in column {column+1}"
        for i in range(self.height - 1, -1, -1):
            if i == 0 and board.matrix[column][i] == 0:
                self.matrix[column][0] = player
                print(f"{notification}, row {self.rows[i]}")
                return
            if board.matrix[column][i] != 0:
                if i == self.height - 1:
                    print(f"COLUMN FULL!\nCan\'t place token in column {i}.")
                else:
                    self.matrix[column][i + 1] = player
                    print(f"{notification}, row {self.rows[i+1]}")
                return

    def checkWin(self):
        # check for win by column
        def checkColumns():
            # go through columns
            for c in range(7):
                column = self.matrix[c]
                for r in range(4):
                    # start points r
                    start = column[r]
                    if start != 0 and all(token == start for token in column[r:r + 4]):
                        print(f"Win by column for player {start}")
                        print(f"{self.rows[r]}{c+1}-{self.rows[r+3]}{c+1}")
                        self.endGame = True
        # check for win by row

        def checkRows():
            # go through rows
            for r in range(self.height - 1, -1, -1):
                # write rows as lists
                row = [self.matrix[c][r] for c in range(7)]
                for c in range(4):
                    start = row[c]
                    if start != 0 and all(token == start for token in row[c:c + 4]):
                        print(f"Win by row for player {start}")
                        print(f"{self.rows[r]}{c+1}-{self.rows[r]}{c+4}")
                        self.endGame = True

        def checkbltrDiagonals():
            for c in range(4):
                for r in range(4):
                    diagonal = [self.matrix[c + x][r + x] for x in range(4)]
                    start = diagonal[0]
                    if start != 0 and all(token == start for token in diagonal):
                        print(f"Win by diagonal bltr for player {start}")
                        print(f"{self.rows[r]}{c+1}-{self.rows[r+3]}{c+4}")
                        self.endGame = True

        def checktlbrDiagonals():
            for c in range(4):
                for r in range(3, 7):
                    diagonal = [self.matrix[c + x][r - x] for x in range(4)]
                    start = diagonal[0]
                    if start != 0 and all(token == start for token in diagonal):
                        print(f"Win by diagonal tlbr for player {start}")
                        print(f"{self.rows[r]}{c+1}-{self.rows[r-3]}{c+4}")
                        self.endGame = True

        checkColumns()
        checkRows()
        checktlbrDiagonals()
        checkbltrDiagonals()

    def checkDraw(self):
        if all(x != 0 for x in self.flatMatrix()):
            return True

    def applyGravity(self):
        for i in range(7):
            self.matrix[i] = [x for x in self.matrix[i] if x != 0]
            self.matrix[i] += [0 for _ in range(7 - len(self.matrix[i]))]

    def rotateLeft(self):
        self.matrix = list(list(x) for x in zip(*self.matrix))[::-1]

    def rotateRight(self):
        self.matrix = list(list(x)[::-1] for x in zip(*self.matrix))

    def gameLoop(self):
        print("---NEW GAME")
        self.drawBoard()
        while self.endGame is False:
            print(f"---Turn {self.turnNumber}:")
            turn = input(f"Player {self.playerTurn}, make your move.\n(1,2,3,4,5,6,7,L,R)\n---")
            if turn not in ["1", "2", "3", "4", "5", "6", "7", "l", "L", "r", "R"]:
                print(f"WRONG INPUT {turn}!\nInput must be L, R or an integer between 1 and 7.")
                continue
            if turn == "L" or turn == "l":
                self.rotateLeft()
                self.applyGravity()
            elif turn == "R" or turn == "r":
                self.rotateRight()
                self.applyGravity()
            elif int(turn) in [1, 2, 3, 4, 5, 6, 7]:
                self.putToken(self.playerTurn, int(turn) - 1)
            self.checkWin()
            if self.endGame is True:
                self.winner = self.playerTurn
                break
            if self.checkDraw():
                break
            self.playerTurn = 1 if self.playerTurn == 2 else 2
            self.turnNumber += 1
            self.drawBoard()
        if self.winner == 0:
            print(f"DRAW!")
        else:
            print(f"Player {self.winner} won in {self.turnNumber} turns!")
        self.drawBoard()
        return self.winner


board = TFBoard()
board.gameLoop()
\$\endgroup\$
2
\$\begingroup\$

As there is no answer yet - some observations in no specific order

you miss the top level main guard which allows the module to be imported

if __name__ == '__main__':
    board = TFBoard()
    board.gameLoop()

your class is not really a class

It is not intended to be copied, compared, it does not communicate with other program parts. It even has a blocking loop. It is just a namespace like container. however modules do already fulfill the namespace. you could safely remove all class and self stuff and run the application like

if __name__ == '__main__':
    init()
    gameLoop()

If you want to design a class you have to remove the gameLoop from it. then you notice there is a little problem with the rotate which forces you to expose internal implementation details to the main loop by requiring a call to applyGravity as well. by the way putToken does the job correctly, one input resulting in one call to the board.

ensure functions/methods do what the name promises

rotate does not rotate the board, but a matrix only. it would make a perfect name for a method of a matrix class. a board method should do the complete job and apply the gravity as well.

reuse what you have

you could insert tokens on the top only and use applyGravity to let it fall down. not only this resembles physics, but also there is less code to test. putToken shows high complexity and repeated code.

handle errors

your putToken does detect an error if a column is full. however it does not forward this to the loop, so the player is turned over.

separate I/O from core

do not do I/O from core functions, but from main loop only. this requires providing state and error information to the loop by return values or raising exceptions. or by providing getters for state. this will immediately lead to better design and testable functions.

there shall be no magic numbers in the code, define constants

when you decide to go for a 9x9 instead of a 7x7 board, there shall be only two edits. you make it worse by having those numbers as attributes, but you do use them only for drawing. when checking for wins, applying gravity or handling input you have raw numbers in the code. this is the worst possible combination, pretend to handle your constants properly in the __init__ but ignoring these later on. seriously, this is an absolute fail. also have a definition for 4 and write correct expressions for all the dependent values subtracting it from height or width.

some python sugar

self.matrix[i] += [0 for _ in range(7 - len(self.matrix[i]))]

may be written shorter (and more readable)

self.matrix[i] += [0] * (7 - len(self.matrix[i]))

learn to write unit tests

python provides built-in unit test support. it is really easy to use and of great use. what you invest in unit testing you get returned in better design, less errors, better maintainability and probably you even save time you use for debugging otherwise. again seriously, you will be a much better programmer when you have done your first little project with unit testing.

finally

my answer is intended to help you, not to intimidate you. try to understand and get better. if you have questions regarding my points, please feel free to ask.

\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer. I made it a class to call it multiple times by another module, for example when I let random players play against each other multiple times. The matrix is the list of lists that stores the game data, the board is the thing that is drawn into the console. The code reuse is totally true, this is something I just dont see in my own code. In case of a full coloumn, the player is not turned over because the player number does not change. There is a continue statement for that case. i would like to go throught a different file if you are up for it. We could use the SO chat \$\endgroup\$ – Tweakimp Feb 16 '18 at 19:49
  • \$\begingroup\$ I'm off in a minute. Feel free to start a chat, I definitely will answer, but unfortunately not now. \$\endgroup\$ – stefan Feb 16 '18 at 20:04
  • \$\begingroup\$ I dont know id this works... chat.stackexchange.com/rooms/73272/tweakimp \$\endgroup\$ – Tweakimp Feb 16 '18 at 20:33

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.