8
\$\begingroup\$

I've made a working Four-in-a-Row game with grid lists and winning system. Both vertically and diagonally, is there some clear improvements to be made or common mistakes which could perhaps shorten it down? Is there any bad code or stupid logic which needs improving?

I haven't gotten to making the diagonal victory but have kept a few ideas in mind. Tips here would also be appreciated.

import os

gw = False

def cls():
    os.system('cls')
#Check vertical 4 in a row.
def check_ver():
    for row in range(6):
        xCounter = 0
        for col in range(5):
            if (grid[col][row] == "X"):
                xCounter += 1
        if xCounter == 4:
            xw()
    for row in range(6):
        oCounter = 0
        for col in range(5):
            if (grid[col][row] == "O"):
                oCounter += 1
        if oCounter == 4:
            ow()
#Check horizontal 4 in a row.
def check_hor():
    for col in range(5):
        xCounter = 0
        for row in range(6):
            if (grid[col][row] == "X"):
                xCounter += 1
            if xCounter == 4:
                xw()
    for col in range(5):
        oCounter = 0
        for row in range(6):
            if (grid[col][row] == "O"):
                oCounter += 1
            if oCounter == 4:
                xw()
#Checks all
def check_all():
    check_ver()
    check_hor()
#Checks for win, prints the game
def p():
    cls()
    check_all()
    for x in range(5):
        print(*grid[x], sep=" | ")
    for x in range(2):
        print(*info[x], sep=" | ")
#Empty prints for victory message
def em():
    for x in range(3):
        print("")
#Victory message and stuff for O
def ow():
    cls()
    em()
    print("O, Wins!".center(16))
    em()
    gw = True
#Victory message and stuff for X
def xw():
    cls()
    em()
    print("X, Wins!".center(16))
    em()
    gw = True
#Grid system
grid = [['.', '.', '.', '.', '.', '.'],
        ['.', '.', '.', '.', '.', '.'],
        ['.', '.', '.', '.', '.', '.'],
        ['.', '.', '.', '.', '.', '.'],
        ['.', '.', '.', '.', '.', '.']]
info = [['==+===+===+===+===+=='],
        ['1', '2', '3', '4', '5', '6']]
#Game
while True:
    if gw == True:
        break
    else:
        p()
        row = int(input("X, column: "))
        row -= 1
        if 0 > row or row > 6:
            continue
        else:
            for x in range(4, -1, -1):
                if grid[x][row] == '.':
                    grid[x][row] = 'X'
                    break
        p()
        row = int(input("O, column: "))
        row -= 1
        if 0 > row or row > 6:
            continue
        else:
            for x in range(4, -1, -1):
                if grid[x][row] == '.':
                    grid[x][row] = 'O'
                    break
\$\endgroup\$
1
  • 2
    \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Feel free to edit and give it a different title if there is something more appropriate. \$\endgroup\$ Dec 16, 2021 at 18:48

2 Answers 2

10
\$\begingroup\$

Stop repeating yourself. You have a lot of repetitive code. A function to check for horizontal win and a similar one for vertical win. Within each of those functions, nearly identical blocks of code for X and then for O. And within the main while-true loop, nearly identical blocks of code to get input for player X and player O. None of that repetition is needed, some of it causes bugs (eg, when X wins the program still asks for more input from O), and you'll probably get good advice from others for eliminating a lot of it. I'm going to focus on one topic: checking for wins.

Checking one row is easy. In these two-dimenional grid situations, rows are typically the easy part, because the grid is naturally organized row by row. So let's start with a simple function to determine whether there is a win in a single row. We can use itertools.groupby to organize the row into each player's contiguous sequences of cells. Then we just find the longest sequence and return the player of that sequence, provided that it is long enough:

from itertools import groupby

def collect_runs(row):
    # Takes a row. Yields all contiguous non-None sequences.
    for player, cells in groupby(row):
        if player:
            yield list(cells)

def check_row_win(row, win_size = 4):
    # Takes a row. Returns player of a winning run.
    runs = sorted(collect_runs(row), key = len)
    if not runs:
        return None
    long_run = runs[-1]
    player = long_run[0]
    return player if len(long_run) >= win_size else None

Checking all rows is also easy. With those building blocks in place, we can check for horizontal wins by either player in one function -- in other words, no need to repeat ourselves:

def check_horizontal_wins(grid):
    for row in grid:
        winner = check_row_win(row)
        if winner:
            return winner
    return None

Use transpose to convert rows to columns. Transposing a grid is easy in Python. If we take our original grid and transpose it, we can use the same row-checking logic to check for both horizontal and vertical wins:

def transpose(grid):
    return [list(tup) for tup in zip(*grid)]

def check_wins(grid):
    # Now it handles horizontal and vertical.
    for row in grid + transpose(grid):
        winner = check_row_win(row)
        if winner:
            return winner
    return None

Use the same mindset to handle diagonal wins. We've had good luck so far by using a simple strategy: rather than repeating code or writing complex logic for every case, we applied a fairly simple data transformation to re-use some basic win-checking logic. With a little experimentation we can figure out a way to convert diagonals to rows. For lack of a better term, I'll call the transformation we need a grid shift:

# Consider a forward-diagonal win.

    . . . x .
    . . x . .
    . x . . .
    x . . . .

# A grid shift converts the diagonal to vertical, which we can handle.
# The process will include padding to fill in the empty quadrants.

    . . . x .
      . . x . .
        . x . . .
          x . . . .

# Now consider a backward-diagonal win.

    . o . . .
    . . o . .
    . . . o .
    . . . . o

# Reverse the rows and it becomes a forward-diagonal, which we can handle.

    . . . . o
    . . . o .
    . . o . .
    . o . . .

Just use row-checking logic for everything. Here is one way to implement the shift transformation, along with the resulting function that can check for all types of wins:

def shift(grid):
    return [
        padding(r) + row + padding(len(row) - r - 1)
        for r, row in enumerate(grid)
    ]

def padding(n):
    return [None for _ in range(n)]

def check_wins(grid):
    all_rows = (
        grid +                           # Horizontal
        transpose(grid) +                # Vertical
        transpose(shift(grid)) +         # Forward diagonal
        transpose(shift(reversed(grid))) # Backward diagonal
    )
    for row in all_rows:
        winner = check_row_win(row)
        if winner:
            return winner
    return None
\$\endgroup\$
2
  • 2
    \$\begingroup\$ Nice answer. Why have you made collect_runs separate to check_row_win? For example changing collect_runs to have if player and len(cells) >= win_size: return player instead. \$\endgroup\$
    – Peilonrayz
    Dec 16, 2021 at 22:27
  • 1
    \$\begingroup\$ @Peilonrayz Probably no good reason other than exposition and mostly habit. For many years I asked variants of this type of question during software engineering interviews and often in those questions we needed to know more about all of the runs than just whether there were 4 in a row. The OP should consider your suggestion seriously, because it is a good simplification. \$\endgroup\$
    – FMc
    Dec 16, 2021 at 23:34
2
\$\begingroup\$

Also bear in mind that os.system('cls') is not platform independent.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ For platform independency, use os.system("cls" if os.name == "nt" else "clear"). \$\endgroup\$ Dec 17, 2021 at 12:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.