Connect 4 on a 4×4 board in Python

Question

I'm having some trouble improving the Connect 4 game that is made in Python. I was wondering if I could get rid of the global statements and pass the variables between parameters and return its values. Also to put all of this in a class. I was wondering if this is possible.

And any more improvements that I could use would be appreciated a lot. Be gentle since I'm a newbie with Python.

grids = [[0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0], [0, 0, 0, 0]]

check = []

user = 1


# THIS WAS VERSION ONE
# def GridBuilder(x,y):
#     Colum_Count_Str = x
#     Colum_Count = int(Colum_Count_Str)
#     Row_Count_Str = y
#     Row_Count = int(Row_Count_Str)
#
#     Grids = [] * Colum_Count
#
#     for Current_Grid_Insert in range (Colum_Count):
#        Grids.insert(Current_Grid_Insert,[0]*Row_Count)
#
#     return Grids


class FullCapacity_error(Exception):
    pass


def user_def():
    #Change the global vartiables into local variables
    #Define them in the parameters of the functions.
    global user
    if user < 2:
        user = 2
    else:
        user = 1
    return user


def FullCapacity():
    global FullCapacity_error
    while True:
        try:
            if grids[3][userInput - 1] != 0:
                raise FullCapacity_error
            else:
                break
        except FullCapacity_error:
            print("This row is full, try another one!")
            confirmed()


def confirmed():
    PlayTheGame = True
    while PlayTheGame:
        try:
            global userInput
            userInput = int(input("\n Input a coin, player " + str(user) + "(1,4)\n"))
            if 5 > userInput > 0:
                PlayTheGame = False
            else:
                print("This number exceeds the board value.")
        except ValueError:
            print("This number exceeds the board value.")


def placement_def():
    for i in range(0, 4):
        counter = 0
        FullCapacity()
        if grids[i][userInput - 1] == 0:
            grids[i][userInput - 1] = int(user)
            print("\n Current Board\n", grids[3], "\n", grids[2], "\n", grids[1], "\n", grids[0])
            break


def check_def():
    global loop
    global check
    for i in range(0, 4):
        for a in range(0, 4):
            check.append(grids[i][a])
        if check == [1, 1, 1, 1] or check == [2, 2, 2, 2]:
            print("player " + str(user) + " has won")
            loop = False
            return loop

        else:
            check = []
    for i in range(0, 4):
        for a in range(0, 4):
            check.append(grids[a][i])
        if check == [1, 1, 1, 1] or check == [2, 2, 2, 2]:
            print("player " + str(user) + " has won")
            loop = False
            return loop

        else:
            check = []


def checkEmpty_def():
    global check
    for i in range(0, 4):
        for a in range(0, 4):
            check.append(grids[i][a])
    if 0 not in check:
        print("full")


def checks_def():
    return check_def() and checkEmpty_def() and diagcheck_def()


def diagcheck_def():
    global loop
    global check
    check = []
    diag = 0
    for i in range(0, 4):
        check.append(grids[diag][diag])
        diag = diag + 1
        if check == [1, 1, 1, 1] or check == [2, 2, 2, 2]:
            print("player " + str(user) + " has won")
            loop = False
            return loop

    check = []
    diag = 3
    diag2 = 0
    for i in range(0, 4):
        check.append(grids[diag][diag2])
        if check == [1, 1, 1, 1] or check == [2, 2, 2, 2]:
            print("player " + str(user) + " has won")
            loop = False
            return loop


loop = True

while loop:
    check_def()
    confirmed()
    placement_def()
    checks_def()
    if not loop:
        break
    user_def() 

Answer 1

Design

You've got the right diagnosis: your functions are poorly designed, such that it is unclear what their inputs and outputs are. We definitely need to fix your functions so that they act the way functions should. Redesigning the code to be object-oriented would not necessarily improve the code further, though.

Let's consider your main loop:

loop = True

while loop:
    check_def()
    confirmed()
    placement_def()
    checks_def()
    if not loop:
        break
    user_def()

Looking at just that code, I can't tell what your functions do. I can't even tell that it's code for a board game. Compare that to the main() function in my solution below:

def main():
    board = [
        [0, 0, 0, 0],
        [0, 0, 0, 0],
        [0, 0, 0, 0],
        [0, 0, 0, 0],
    ]
    print_board(board)
    for player in itertools.cycle([1, 2]):
        make_move(board, player)
        print_board(board)
        if is_winner(board, player):
            print("Player {0} has won".format(player))
            break
        if is_board_full(board):
            print("Board is full")
            break

if __name__ == '__main__':
    main()

This code makes it clear that it's a board game in which two players take turns making a move. Each function has:

• A purpose, and the name tells you what it does. check_def doesn't mean anything, but is_winner does. The fact that the name starts with "is_…" implies that it's a function that returns either True or False.
• A useful return value. If is_winner(…) returns True, then the if statement in the loop can use that information to end the game. You don't want the information to be passed back indirectly by setting a global loop variable.
• Inputs passed as parameters. What information does is_winner(…) need to do its job? Obviously, it needs to know the state of the board. It's also helpful to know which player just moved, so that it only needs to consider the most recent player's coins.

To help enforce these good habits, you should write a docstring for every function that explains the purpose, the inputs, and the return value.

Bug

Your checks_def() function, which is intended to detect the end of the game, is buggy:

def checks_def():
    return check_def() and checkEmpty_def() and diagcheck_def()

check_def() (which detects horizontal and vertical wins) never returns True: it can either return False, or fall off the end of the function and implicitly return None. Therefore, due to the way short-circuit evaluation works with the and operator, the code checkEmpty_def() (to detect a tie) and diagcheck_def() (to detect a diagonal win) will never be executed.

The logic in that line is further wrong in that you should check for diagonal wins before detecting that the board is full, because the last coin that fills the board might be a winning move.

Expressiveness

Python has some very powerful techniques to write loops with a lot less code. In the solution below, my is_winner(…) and is_board_full(…) functions make use of the built-in functions all() and any() with generator expressions.

zip(*board) is a handy idiom for transposing row and columns.

Suggested solution

from itertools import cycle

def print_board(board):
    """
    Print the board.
    """
    print("\nCurrent board:")
    for row in reversed(board):
        print(row)

def make_move(board, player):
    """
    Ask the player for a column, and mark the move on the board if it is a
    legal move.  If it is an illegal move, ask again.
    """
    while True:
        try:
            c = -1 + int(input("Input a coin, player {0} (1-4): ".format(player)))
            if not (0 <= c < 4):
                print("Column number is out of range.")
            top_row = next(row for row in board if row[c] == 0)
            top_row[c] = player
            break
        except ValueError:
            print("Illegal move.")
        except StopIteration:
            print("This column is full.  Try another one!")

def is_winner(board, player):
    """
    Return True if a row, column, or diagonal consisting of four entries for
    the given player.
    """
    return (
        any(all(c == player for c in row) for row in board) or
        any(all(r == player for r in col) for col in zip(*board)) or
        (board[0][0] == board[1][1] == board[2][2] == board[3][3] == player) or
        (board[0][3] == board[1][2] == board[2][1] == board[3][0] == player)
    )

def is_board_full(board):
    """
    Return True if all elements of the board are non-zero.
    """
    return all(all(row) for row in board)

def main():
    board = [
        [0, 0, 0, 0],
        [0, 0, 0, 0],
        [0, 0, 0, 0],
        [0, 0, 0, 0],
    ]
    print_board(board)
    for player in cycle([1, 2]):
        make_move(board, player)
        print_board(board)
        if is_winner(board, player):
            print("Player {0} has won".format(player))
            break
        if is_board_full(board):
            print("Board is full")
            break

if __name__ == '__main__':
    main()