8
\$\begingroup\$

I recently read the book Clean Code and I also did some research on the SOLID principles. I'm looking for general feedback on if I was able to transpose the examples (written in Java) to Python while still maintaining a "Pythonic" program.

I wrote a simple TicTacToe game in Python using Tkinter as the GUI and I tried to strike a balance between readable, clean code and avoiding code cluttering with useless functions (get(), set(), isEmpty() and other 1-liners)

Having a generic BoardGUI class specializing in TicTacToeBoardGUI is my attempt at the open/close principle.

The 3 modules (models, views, game) implement a minimalistic MVC design where GameApplication controls the flow and events.

The part I don't know how to handle is using polymorphism to have different levels of AI (right now the AI only checks a random empty box)

http://www.filedropper.com/tictactoe (~200 lines total)

main.py

#!/usr/bin/env

import Tkinter as tk
from game import GameApplication


def convert_images():
    """Returns a dictionary with the images converted into PhotoImages Tkinter can use"""
    img_dict = {}
    img_dict.update({'cross': tk.PhotoImage(file='images/cross.gif')})
    img_dict.update({'circle': tk.PhotoImage(file='images/circle.gif')})
    return img_dict


def main():
    root = tk.Tk()
    root.title('Tic-Tac-Toe v1.0')

    images = convert_images()
    GameApplication(root, images)

    root.mainloop()

if __name__ == '__main__':
    main()

models.py:

#!/usr/bin/env
from collections import namedtuple


class TicTacToeBoard():
    """
    Keeps track of the status of the boxes of a standard 3x3 TicTacToe game

    I don't particularly like the victory, draw and _find_lines functions, any better solutions?
    """

    ROWS = 3
    COLUMNS = 3
    EMPTY = 0
    O = 1
    X = 2

    def __init__(self):
        self.boxes = {(column, row): namedtuple('box', 'value') \
                    for row in range(TicTacToeBoard.ROWS) \
                        for column in range(TicTacToeBoard.COLUMNS)}

        for __, items in self.boxes.items():
            items.value = TicTacToeBoard.EMPTY

        self._lines = self._find_all_lines()

    def victory(self):
        for line in self._lines:
            if self.boxes[line[0]].value == self.boxes[line[1]].value == self.boxes[line[2]].value:
                if self.boxes[line[0]].value != TicTacToeBoard.EMPTY:
                    return True

        return False

    def draw(self):
        for __, box in self.boxes.items():
        if box.value == TicTacToeBoard.EMPTY:
            return False

        return True

    def _find_all_lines(self):
        lines = []

        for x in range(3):
            lines.append(((0, x), (1, x), (2, x)))
            lines.append(((x, 0), (x, 1), (x, 2)))
        lines.append(((0, 0), (1, 1), (2, 2)))
        lines.append(((0, 2), (1, 1), (2, 0)))

        return lines

views.py:

#!/usr/bin/env

import Tkinter as tk
from math import ceil


class BoardGUI(tk.LabelFrame):
    """
    Questions:
        Does this class respects the open/close principle?
        Am I reinventing the wheel by transforming the coordinates to row and
        columns or Tkinter.canvas can do that already?

    A generic board game of arbitrary size, number of columns and number of rows.
    It can be inherited to serve as a point and click GUI for different board games.
    The mouse coordinates should be carried by an event passed in parameter to the functions

    Boxes are accessed with the tags kw from a Tkinter canvas
    They are nothing but drawings; actual rows and columns are computed based on mouse position and size

    Needs Tkinter compatible images to draw in boxes
    Coordinates for the boxes are either in form of (box_column, box_row) or (box_top_left_x, box_top_left_y)
    """

    def __init__(self, root, rows, columns, width, height, **options):
        tk.LabelFrame.__init__(self, root, **options)

        self._rows = rows
        self._columns = columns
        self._width = width
        self._height = height

        self.canvas = tk.Canvas(self, width=self._width, height=self._height)

        self.canvas.pack(side=tk.TOP, fill=tk.BOTH, expand=tk.TRUE)
        self._draw_boxes()

    def get_box_coord(self, event):
        return self._convert_pixels_to_box_coord(event.x, event.y)

    def draw_image_on_box(self, coord, image):
        (x, y) = self._convert_box_coord_to_pixels(coord)
        self.canvas.create_image((x, y), image=image, anchor=tk.NW)

    def draw_message_in_center(self, message):
        self.canvas.create_text(self._find_board_center(), text=message, font=("Helvetica", 50, "bold"))

    def _draw_boxes(self):
        [self.canvas.create_rectangle(self._find_box_corners((column, row))) \
            for row in range(self._rows) \
                for column in range(self._columns)]

    def _find_box_corners(self, coord):
        boxWidth, boxHeight = self._find_box_dimensions()
        column, row = coord[0], coord[1]

        topLeftCorner = column * boxWidth
        topRightCorner = (column * boxWidth) + boxWidth
        bottomLeftCorner = row * boxHeight
        bottomRightCorner = (row * boxHeight) + boxWidth

        return (topLeftCorner, bottomLeftCorner, topRightCorner, bottomRightCorner)

    def _find_board_center(self):
        return ((self._width / 2), (self._height / 2))

    def _find_box_dimensions(self):
        return ((self._width / self._columns), (self._height / self._rows))

    def _convert_box_coord_to_pixels(self, coord):
        boxWidth, boxHeight = self._find_box_dimensions()
        return ((coord[0] * boxWidth), (coord[1] * boxHeight))

    def _convert_pixels_to_box_coord(self, x, y):
        column = ceil(x / (self._height / self._columns))
        row = ceil(y / (self._width / self._rows))
        return (column, row)


class TicTacToeBoardGUI(BoardGUI):
    """
    Allows you to draw circles or crosses on a 3x3 square gaming board

    Tkinter needs to keep a reference of the images to loop. It's better to
    initiate them elsewhere and pass them in parameters even if they seem like
    they should be internal attributes.
    """

    ROWS = 3
    COLUMNS = 3

    def __init__(self, root, width, height, images):
        BoardGUI.__init__(self, root, TicTacToeBoardGUI.ROWS, TicTacToeBoardGUI.COLUMNS, width, height)

        self.xSymbol = images['cross']
        self.oSymbol = images['circle']

    def draw_x_on_box(self, coord):
        self.draw_image_on_box(coord, self.xSymbol)

    def draw_o_on_box(self, coord):
        self.draw_image_on_box(coord, self.oSymbol)

game.py:

#!/usr/bin/env

import Tkinter as tk
from views import TicTacToeBoardGUI
from models import TicTacToeBoard
from random import shuffle


class GameApplication(tk.Frame):
    """
    Serves as a top level Tkinter frame to display the game and as a controller to connect
    the models and the views.

    I'm willingly giving this class 2 tasks because the controller as a single event to connect
    and the purpose of this exercise isn't the MVC pattern but the SOLID and clean code principles

    It simply instantiates the models and views necessary to play a TicTacToe game and interpret a left
    mouse click as a move from the Human player
    """

    CANVAS_WIDTH = 300
    CANVAS_HEIGHT = 300

    def __init__(self, root, images, **options):
        tk.Frame.__init__(self, root, **options)

        self._boardModel = TicTacToeBoard()

        self._boardView = TicTacToeBoardGUI(root, GameApplication.CANVAS_WIDTH, GameApplication.CANVAS_HEIGHT, images)
        self._boardView.pack()

        self._connect_click_event()

        if self._ai_goes_first():
            self._ai_plays()

    def _player_plays(self, event):
        coord = self._boardView.get_box_coord(event)

        if self._boardModel.boxes[coord].value != self._boardModel.EMPTY:
            return

        self._boardModel.boxes[coord].value = self._boardModel.X
        self._boardView.draw_x_on_box(coord)

        self._ai_plays()

    def _ai_plays(self):
        #Simply checks a random empty box on the board;
        #The minmax algorithm could be used to never lose but that's no fun
        for coord, box in self._boardModel.boxes.items():
            if box.value == self._boardModel.EMPTY:
                self._boardModel.boxes[coord].value = self._boardModel.O
                self._boardView.draw_o_on_box(coord)
                break

        self._check_end_game_status()

    def _check_end_game_status(self):
        if self._boardModel.victory():
            self._boardView.draw_message_in_center("Victory!")
        elif self._boardModel.draw():
            self._boardView.draw_message_in_center("Draw!")

    def _ai_goes_first(self):
        result = [True, False]
        shuffle(result)
        return result[0]

    def _connect_click_event(self):
        self._boardView.canvas.bind("<Button-1>", self._player_plays)
\$\endgroup\$
  • 1
    \$\begingroup\$ Please check the indentation; it is off at least in TicTacToeBoard.draw. \$\endgroup\$ – Janne Karila Jan 8 '14 at 7:36
  • 1
    \$\begingroup\$ collections.namedtuple does not work like you think it does, please check docs. \$\endgroup\$ – Janne Karila Jan 8 '14 at 13:12
4
\$\begingroup\$

Looks to me that you want to have a box with various buttons that allows you to select, say, easy, medium, or hard. Return that and then say

if AIChoose == "Easy":
    getEasyAIMove()
elif AIChoose == "Medium":
    GetMedAIMove()
elif AIChoose == "Hard":
    GetHardAIMove()

Now, that's obviously the easy part, but a Tic-Tac-Toe AI is actually not at all hard:

def GetHardAIMove():
    # first check if you're about to win. If you are, move there
    # now check if your opponent is about to win. If he is, block him
    # check if any of the corners are free. If they are, go to a random one of them
    # check if the middle is free. If it is, go to it
    #otherwise, go to a random open spot
def GetMedAIMove():
    # here's where we cheat. generate a random number between 1 and 3. if its <= 2, GetHardAIMove. otherwise, go easy.
def GetEasyAIMove():
    # If you're about to win, go there
    # If you're about to lose, go there
    # Otherwise, go to a random spot.

See? This is a quick and painless AI that can beat decent players who don't know how it's made about 50-60% of the time.

EDIT: Since Tic-Tac-Toe can always be won or tied by the player that moves first, you may wish to add this into the AI by doing something like this:

def GetHardAIMove():
    # if you move first:
        # after making sure you or your opponent isn't about to win:
        # move to a corner
        # then move to the corner opposite that
        # then move to a random corner
        # at this point, you have either won or tied
    # first check if you're about to win. If you are, move there
    # now check if your opponent is about to win. If he is, block him
    # check if any of the corners are free. If they are, go to a random one of them
    # check if the middle is free. If it is, go to it
    #otherwise, go to a random open spot
\$\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.