OOP TictacToe with Tkinter

This is my implementation of a Tic Tac Toe game with Tkinter GUI. So far I've set up the game to play with another player. For an interview coming up, I am suppose to build additional feature such as play with AI.

I would like some general feedback on my code. Does my code smell? Can I organize the Player, Board, GameApp objects and methods better? Is permutation on sets a bad idea? Any suggestion is welcomed.

So far I've sent up my classes and methods in this order:

Player class

Attributes:

• name as string

• color as string

• selected_sq as empty set

Methods:

None

Board class

Attributes:

• parent (instance of Tkinter class)

• sq_size

• color

• container, canvas - (from Tkinter class to draw board)

• winning_combo - (dictionary of winning combo in relation to location of rowcol)

Methods:

• get_unused_squares_dict(self)

• reset_unused_squares_dict(self)

• draw_board(self)

• get_row_col(self, evt)

• loor_of_row_col(self, col, rw)

• convert_to_key(self, col_floor, row_floor)

• find_coords_of_selected_sq(self, evt)

• color_selected_sq(self, evt, second_corner_col, second_corner_row, player_color)

GameApp class

Attributes:

• parent (instance of Tkinter class)

• board

• player1

• player2

• computer

Methods:

• initialize_buttons

• show_menu

• init_two_players_game

• restart

• play

• check_for_winner

• show_game_result

• add_player_to_sq

• delete_used_sq

• init_computer_game (later feature)

• play_computer (later feature)

```
#-----------------------------------------------------------------------------
# Name: Tic Tac Toe
# Purpose: Simple game of tic tac toe with tkinter library for GUI
#-----------------------------------------------------------------------------

import tkinter
import random
from itertools import permutations

class Player:
"""
Player class to be used in the Game obj

Attributes:
name: tex

There's quite a lot of code here, so I'm just going to review one of your methods, check_for_winner. You'll see that there's plenty here for one answer. Maybe some other reviewers can look at the rest of the code.

-
There's no docstring. It's useful to write docstrings for all your methods, because (i) it's easy to forget what a method does and having a docstring will help remind you; (ii) you can check the docstring against the code to see if you've implemented it correctly; (iii) writing a specification forces you to think about whether that specification makes sense.

-
In the case of check_for_winner, the docstring would have to look something like this:

"""Given that the player whose name is player_name has played at
the squares in the sequence player_sq, determine if that
player has won, or if the game has ended in a tie, and if
either of these conditions applies, create a label with the
appropriate game over message, wait for the restart button to
be pressed, and restart the game."""

This is pretty complicated specification, and not something that you might guess from the name of the method. In particular it shows a lack of model–view separation. This is an important technique in user interface programming, where you separate the code into a model, which describes the abstract data structures and logic (here, the tic-tac-toe board and its rules), and a view, which presents these data structures to the user via some kind of interface.

There are several benefits of separation, including (i) keeping things simple and maintainable; (ii) testing the model; (iii) easily replacing one kind of view with another (for example, using a different user interface toolkit).

So here we would want to divide check_for_winner into two pieces: a method on the Board class that determines the state of the board (game not over? game tied? win for player 1? win for player 2?); and a method on the Game class that deals with the result.

-
This line appears twice:

self.restart

but has no effect: it's missing the parentheses that are needed for a method call. Remove it.

-
There's a bug in the win determination. The code only checks to see if the player has won if len(self.board.unused_squares_dict) > 0. But this means that if a player wins by playing in the last empty square on the board, this win won't be detected.

-
The win determination is inefficient: (i) it loops over all combinations of three places where the player has played, even though many of these triples can't possibly result in a win; (ii) it searches for each triple in the list _winning_combos, which might require comparing against every element in the list; (iii) it repeatedly converts the combination into a set, once for each element in the _winning_combos list; (iv) it keeps going even after it has determined the winner.

Tic-tac-toe is such a small game that this inefficiency doesn't matter very much. But there are two reasons to be concerned: (i) thinking about efficiency is an important skill, so it's good to practice that skill in easy situations like this; (ii) you say that your next step is to add computer play, which might involve game tree search and when many positions are being searched, the speed of evaluation of those positions is important.

A couple of simple improvements: (i) use the subset operation `

StackExchange Code Review Q#91353, answer score: 5