4
\$\begingroup\$

I've decided to use some of my free time to create a simple game in Python. I chose Tic-Tac-Toe, but I decided not to make it too trivial. I made my game support any board size. I would greatly appreciate a code review.

## Tic-tac-toe game by Mateon1

try:
    raw_input
except NameError:
    import sys
    print >> sys.stderr, "This program supports Python 2 only."
    sys.exit(1)

def prompt(question, check_func=lambda i: i.lower() == "y"):
    """Prompts the user with `question`
Awaits until check_func(<user_input>) returns a non-None value and returns it"""
    while True:
        value = check_func(raw_input(question))
        if value is not None:
            return value

def check_win(board):
    "Checks if there is a win condition, returns who won or False"
    width = len(board[0])
    height = len(board)

    # Horizontal
    for y in xrange(height):
        if board[y][0] != 0 and all(board[y][i] == board[y][0] for i in xrange(1, width)):
            return board[y][0]

    # Vertical
    for x in xrange(width):
        if board[0][x] != 0 and all(board[i][x] == board[0][x] for i in xrange(1, height)):
            return board[0][x]

    if width == height:  # Diagonals only work if the board is square
        # \ diagonal
        if board[0][0] != 0 and all(board[i][i] == board[0][0] for i in xrange(1, width)):
            return board[0][0]

        # / diagonal
        if board[0][-1] != 0 and all(board[i][-1 - i] == board[0][-1] for i in xrange(1, width)):
            return board[0][-1]
    return False


def f_positive(string):
    try:
        val = int(string)
        if val > 0: return val
    except ValueError:
        pass
    # Implicit `return None` at the end of the function

def f_coords(board):
    "Returns function handling user input for board coordinates"
    def f(i):
        i = i.split(" ")
        if len(i) != 2: return
        x = f_positive(i[0])
        y = f_positive(i[1])
        if x is None or y is None: return
        if board[y - 1][x - 1] != 0: return
        return (x - 1, y - 1)
    return f

def print_board(board):
    """Prints the game board.
Output example:

      1  2  3
    +--+--+--+
  1 |<>|  |<>|
    +--+--+--+
  2 |<>|><|  |
    +--+--+--+
  3 |><|  |  |
    +--+--+--+"""

    def cell(value):
        "Returns a 2-character representation of the cell"
        if value == 0:
            return "  "
        elif value == 1:
            return "<>" # circle
        elif value == 2:
            return "><" # cross
        raise ValueError("Invalid cell value: %r" % value)

    width = len(board[0])
    height = len(board)
    text = [] # Print is annoying, build a buffer instead

    text.append("     ")
    for i in xrange(width):
        text.append("%2d " % (i + 1,))
    text.append("\n")

    for y in xrange(height):
        text.append("    " + "+--" * width + "+\n") # separator
        text.append("%3d " % (y + 1,))
        for x in xrange(width):
            text.append("|" + cell(board[y][x]))
        text.append("|\n")
    text.append("    " + "+--" * width + "+")

    print "".join(text)


def play(board_size=(3, 3)):
    board = [[0 for x in xrange(board_size[0])] for y in xrange(board_size[1])]
    # board[y][x]:
    # * 0 - empty
    # * 1 - <> // circle
    # * 2 - >< // cross
    turn = 0
    while turn < board_size[0] * board_size[1] and not check_win(board):
        print "Player %d's (%s) turn!" % (turn % 2 + 1, ("circle", "cross")[turn % 2])
        print_board(board)
        (x, y) = prompt("Please enter board coordinates to place your piece on.\n(example: `2 3` for 2nd column 3rd row): ", f_coords(board))
        board[y][x] = turn % 2 + 1
        turn += 1

    print "=== Game over! ==="
    print_board(board)

    winning_player = check_win(board)
    if not winning_player:
        print "The game was a tie!"
    else:
        print "Player %d (%s) won!" % (winning_player, ("circle", "cross")[winning_player - 1])


if __name__ == "__main__":
    # Python needs a damn do..while construct
    # Can't avoid repeating myself here.
    board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                  prompt("Please ented tic-tac-toe board height: ", f_positive))
    play(board_size=board_size)
    while prompt("Would you like to play again? (y/N) "):
        board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                      prompt("Please ented tic-tac-toe board height: ", f_positive))
        play(board_size=board_size)
\$\endgroup\$
6
\$\begingroup\$

This won't work:

try:
    raw_input
except NameError:
    import sys
    print >> sys.stderr, "This program supports Python 2 only."
    sys.exit(1)

The program would crash with a SyntaxError during compilation before being able run this. I get

  File "p.py", line 104
    print "".join(text)
           ^
SyntaxError: invalid syntax

One way of doing this check is to have a file like

from __future__ import print_function

try:
    raw_input
except NameError:
    import sys
    print("This program supports Python 2 only.", file=sys.stderr)
    sys.exit(1)

import the_real_program
the_real_program.main()

This program has to support both versions syntactically but can be much smaller.

Personally, though, it looks trivial to get this running on Python 3; use

from __future__ import print_function

try:
    input = raw_input
    range = xrange
except NameError:
    pass

swap the usages and you're done. I suggest you do so; Python 3 is way better anyway.

prompt's docstring should be indented properly and IMHO you shouldn't use backticks. Being pedantic, it should also be phrased as an imperative. Out of the several valid styles, I prefer:

"""
Prompt the user with 'question'.

Await until check_func(<user_input>) returns a non-None value and return it
"""

check_win's docstring should use tripple-quotes:

"""Check if there is a win condition, return who won or False."""

IMHO diagonals shouldn't only work for square boards; this should be a valid win

      1  2  3  4 
    +--+--+--+--+
  1 |<>|  |  |><|
    +--+--+--+--+
  2 |  |<>|  |><|
    +--+--+--+--+
  3 |  |  |<>|  |
    +--+--+--+--+

given that this is also a valid win:

      1  2  3  4 
    +--+--+--+--+
  1 |<>|  |  |><|
    +--+--+--+--+
  2 |<>|  |  |><|
    +--+--+--+--+
  3 |<>|  |  |  |
    +--+--+--+--+

Although this is a gameplay issue so I won't mess with it.

As written, the checks are largely repetitive so I'd suggest extracting it into an operation over indices:

def lines(width, height):    
    for y in range(width):
        yield [(x, y) for x in range(height)]

    for x in range(height):
        yield [(x, y) for y in range(width)]

    if width == height:
        yield [(x,  x) for x in range(width)]
        yield [(x, -x) for x in range(width)]

def check_win(board):
    """Check if there is a win condition, return who won or False."""
    width = len(board[0])
    height = len(board)

    for line in lines(width, height):
        x, y = line[0]
        first = board[x][y]

        if first and all(board[i][j] == first for i, j in line):
            return first

    return False

It's also more typical to return None on failure.

As Caridorc says, return None is much better than # Implicit 'return None' at the end of the function. f_positive should also use try's else clause to minimize the area under it:

def f_positive(string):
    try:
        val = int(string)
    except ValueError:
        pass
    else:
        if val > 0: return val

    return

Sadly there is not try ... elif ;). I would personally invert the logic to an early return:

def f_positive(string):
    try:
        val = int(string)
    except ValueError:
        return

    if val <= 0:
        return

    return val

f_coords is too tightly spaced. Lines are cheap, here are a few for free:

def f_coords(board):
    """Get function handling user input for board coordinates."""
    def f(i):
        i = i.split(" ")
        if len(i) != 2:
            return

        x = f_positive(i[0])
        y = f_positive(i[1])

        if x is None or y is None:
            return

        if board[y - 1][x - 1] != 0:
            return

        return (x - 1, y - 1)
    return f

i is to short, I suggest user_input. finner would be better, traditional naming. You can shorten this to

def f_coords(board):
    """Get function handling user input for board coordinates."""
    def inner(user_input):
        try:
            x, y = map(int, user_input.split(" "))
        except ValueError:
            return

        x -= 1; y -= 1

        if x < 0 or y < 0 or board[y][x]:
            return

        return x, y

    return inner

although you're not checking for large numbers; board[y][x] can crash. Add that check:

def f_coords(board):
    """Get function handling user input for board coordinates."""
    def inner(user_input):
        try:
            x, y = map(int, user_input.split(" "))
        except ValueError:
            return

        x -= 1; y -= 1

        if not (0 <= x < len(board) and 0 <= y < len(board[0])):
            return

        if board[y][x]:
            return

        return x, y
    return inner

print_board's docstring needs indenting too.

Personally I'd use () for a circle; it looks much rounded and easier to distinguish. Your comments for

elif value == 1:
    return "<>" # circle
elif value == 2:
    return "><" # cross

are bad; if the programmer needs commenting then the user is going to be confused.

The cell function should just use a dictionary:

def cell(value):
    """Get a 2-character representation of a cell."""
    return {0: "  ", 1: "()", 2: "><"}[value]

I don't know what you mean by print is annoying. If you have encountered difficulty with something, a good comment will explain what is problematic - not just that it is. Hopefully you'll find the new one more capable:

def cell(value):
    """Get a 2-character representation of a cell."""
    return {0: "  ", 1: "()", 2: "><"}[value]

width = len(board[0])
height = len(board)

numbers = ["{:2}".format(i) for i in range(1, width+1)]
separator = "    " + "+--" * width + "+"

# Output
print("    ", *numbers)

for i, row in enumerate(board, 1):
    print(separator)
    print("{:3} |{}|".format(i, "|".join(map(cell, row))))

print(separator)

In play, board's initialization can be simplified:

board = [[0] * board_size[0] for y in range(board_size[1])]

I would also do

width, height = board_size
board = [[0] * width for y in range(height)]

while turn < width * height and check_win(board) is None: should be a for and a check:

for turn in range(width * height):
    if check_win(board) is not None:
        break

Your %s formatting in print would be better using .format. Also, use %s over %d unless you actually need the specific %d operator.

("circle", "cross")[turn % 2]

should be

"cross" if turn % 2 else "circle"

turn % 2 should be extracted into a player variable since it's used quite a bit. I would even change the loop to

for player_turn in islice(cycle((1, 2)), width * height):

Your prompt could use some line wrapping

    x, y = prompt(
        "Please enter board coordinates to place your piece on.\n"
        "(example: `2 3` for 2nd column 3rd row): ",
        f_coords(board)
    )

Finally, the

# Python needs a damn do..while construct
# Can't avoid repeating myself here.

is trivially solved with break. Look at this isomorphism:

while True:           do {
    things()              things();
    if not x: break   } while (x)

A great thing about the while True version is that it's more flexible! Python doesn't need do ... while.

Here's the code:

while True:
    board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                  prompt("Please ented tic-tac-toe board height: ", f_positive))
    play(board_size=board_size)

    if not prompt("Would you like to play again? (y/N) "):
        break

Finally, throw that in main and make a get_shape(board) -> width, height function and I'm done.

Here's the code

# encoding: utf-8

## Tic-tac-toe game by Mateon1

from __future__ import print_function

from itertools import chain, cycle, islice

try:
    input = raw_input
    range = range
except NameError:
    pass

def get_shape(board):
    return len(board[0]), len(board)

def prompt(question, check_func=lambda i: i.lower() == "y"):
    """
    Prompt the user with 'question'.

    Await until check_func(<user_input>) returns a non-None value and return it
    """
    while True:
        value = check_func(input(question))
        if value is not None:
            return value

def lines(width, height):    
    for y in range(width):
        yield [(x, y) for x in range(height)]

    for x in range(height):
        yield [(x, y) for y in range(width)]

    if width == height:
        yield [(x,  x) for x in range(width)]
        yield [(x, -x) for x in range(width)]

def check_win(board):
    """Check if there is a win condition, return who won or None otherwise."""
    width, height = get_shape(board)

    for line in lines(width, height):
        x, y = line[0]
        first = board[x][y]

        if first and all(board[i][j] == first for i, j in line):
            return first

    return None

def f_positive(string):
    try:
        val = int(string)
    except ValueError:
        return

    if val <= 0:
        return

    return val

def f_coords(board):
    """Get function handling user input for board coordinates."""
    width, height = get_shape(board)

    def inner(user_input):
        try:
            x, y = map(int, user_input.split(" "))
        except ValueError:
            return

        x -= 1; y -= 1

        if not (0 <= x < width and 0 <= y < height):
            return

        if board[y][x]:
            return

        return x, y

    return inner

def print_board(board):
    """
    Print the game board.

    Output example:

          1  2  3
        +--+--+--+
      1 |()|  |()|
        +--+--+--+
      2 |()|><|  |
        +--+--+--+
      3 |><|  |  |
        +--+--+--+
    """

    def cell(value):
        """Get a 2-character representation of a cell."""
        return {0: "  ", 1: "()", 2: "><"}[value]

    width, height = get_shape(board)

    numbers = ["{:2d}".format(i) for i in range(1, width+1)]
    separator = "    " + "+--" * width + "+"

    # Output
    print("    ", *numbers)

    for i, row in enumerate(board, 1):
        print(separator)
        print("{:3d} |{}|".format(i, "|".join(map(cell, row))))

    print(separator)

def play(board_size=(3, 3)):
    width, height = board_size
    board = [[0] * width for y in range(height)]
    # board[y][x]:
    # * 0 - empty
    # * 1 - () // circle
    # * 2 - >< // cross
    for player_turn in islice(cycle((1, 2)), width * height):
        if check_win(board) is not None:
            break

        print("Player {}'s ({}) turn!".format(player_turn, "circle" if player_turn == 1 else "cross"))

        print_board(board)
        x, y = prompt(
            "Please enter board coordinates to place your piece on.\n"
            "(example: `2 3` for 2nd column 3rd row): ",
            f_coords(board)
        )

        board[y][x] = player_turn

    print("=== Game over! ===")
    print_board(board)

    winning_player = check_win(board)
    if winning_player is None:
        print("The game was a tie!")
    else:
        print("Player {} ({}) won!".format(winning_player, "circle" if winning_player == 1 else "cross"))

def main():
    while True:
        board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                      prompt("Please ented tic-tac-toe board height: ", f_positive))
        play(board_size=board_size)

        if not prompt("Would you like to play again? (y/N) "):
            break

if __name__ == "__main__":
    main()
\$\endgroup\$
  • \$\begingroup\$ Great answer, I didn't know about the itertools functions used here. I'll need to look into them. Accepting. \$\endgroup\$ – Mateon1 Jan 9 '15 at 15:59
3
\$\begingroup\$

First, it is vital not to confuse the "game" with its usual "user interface" (the board) because the "game" needs an efficient internal (abstract syntax) representation, whereas the board could take many different representations (or concrete syntaxes).

If these comments (so far) seem vague, try to think how many times after every (valid) move you would need to check using an array (or abstract syntax) if it was a winning move?

If the data structures (abstract syntax) were sets (well, power sets), you could express such tests for valid and winning moves in terms of simple set expressions:

valid_move = current_move not in (x_moves | y_moves)

The distinction between concrete and abstract syntax is vital.

\$\endgroup\$
  • \$\begingroup\$ I made a few formatting changes to this answer and updated the code to Python code. I hope this is OK. \$\endgroup\$ – Veedrac Jan 9 '15 at 22:24
2
\$\begingroup\$

Do not print >> sys.stderr, "This program supports Python 2 only." you should raise an exception instead raise RuntimeError("This program supports Python 2 only.")


# Python does not need a do..while construct
# I can avoid repeating myself here.

def get_size_and_play():
    board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                  prompt("Please ented tic-tac-toe board height: ", f_positive))
    play(board_size=board_size)

if __name__ == "__main__":
    get_size_and_play()
    while prompt("Would you like to play again? (y/N) "):
        get_size_and_play()

You have a comment # Implicit return None at the end of the function writing this makes it explicit, I would:

  1. Just remove the comment (not so good)
  2. Actually add the code return None (better)
\$\endgroup\$
  • 2
    \$\begingroup\$ This isn't sufficient; the other prints will cause a SyntaxError regardless of the check. \$\endgroup\$ – Veedrac Jan 9 '15 at 13:16
  • \$\begingroup\$ I don't understand, the Exception will halt the programme. \$\endgroup\$ – Caridorc Jan 9 '15 at 13:25
  • 2
    \$\begingroup\$ @Caridorc The program will be halted before the code to throw the exception is ran - print x is a syntax error in Python 3. As for the encapsulation of the code in a function as a do..while replacement, I do not like it personally, it feels like a hack - more complex than it should be. \$\endgroup\$ – Mateon1 Jan 9 '15 at 13:35
  • \$\begingroup\$ A script will not even start to execute if there is a syntax error anywhere in it. \$\endgroup\$ – Janne Karila Jan 9 '15 at 13:55
  • \$\begingroup\$ This are all great points, looking back this answer is not good, shiould I delete it? \$\endgroup\$ – Caridorc Jan 9 '15 at 13:55
1
\$\begingroup\$

About this:

# Python needs a damn do..while construct
# Can't avoid repeating myself here.

Yes you can. Change "Would you like to play again? (y/N)" to "Start a new game? (y/N)". Then extract

board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                      prompt("Please ented tic-tac-toe board height: ", f_positive))
play(board_size=board_size)

into some method, call it start_new_game or something. This results in:

if __name__ == "__main__":
    while prompt("Start a new game? (y/N)"):
         start_new_game()
\$\endgroup\$
1
\$\begingroup\$

You could also re-write your starting loop like this:

if __name__ == "__main__":
    first_game = True
    while first_game or prompt("Would you like to play again? (y/N) "):
        board_size = (prompt("Please enter tic-tac-toe board width:  ", f_positive),
                      prompt("Please ented tic-tac-toe board height: ", f_positive))
        play(board_size=board_size)
        first_game = False
\$\endgroup\$
  • \$\begingroup\$ The usual way to do this (as others have commented) is just while True: do_stuff(); if not 'y' in input("do it again? ").lower(): break \$\endgroup\$ – Adam Smith Jan 9 '15 at 18:53

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.