Skip to main content
6 of 6
added 101 characters in body
Justin
  • 2.6k
  • 3
  • 21
  • 59

#Code readability and style

I would recommend you have a look at PEP 8, which is Python's official style guide.

Let's introduce you to f-strings -

To create an f-string, prefix the string with the letter “ f ”. The string itself can be formatted in much the same way that you would with str.format(). f-strings provide a concise and convenient way to embed python expressions inside string literals for formatting.

So, I would write these three statements -

# rest of the code
def __str__(self):
    return "Player {}".format(self.type)
# rest of the code

position = input("{} turn, what's your move? ".format(player))
# rest of the code
print("{} is the Winner!".format(player))

Like this -

# rest of the code
def __str__(self):
    return f"Player {self.type}"
# rest of the code    

position = input(f"{player} turn, what's your move? ")
# rest of the code
print(f"{player} is the Winner!")

See how concise it can get?


From PEP 8 -

PEP 257 describes good docstring conventions. Note that most importantly, the """ that ends a multiline docstring should be on a line by itself -

"""Return a foobang

Optional plotz says to frobnicate the bizbaz first.
"""

For one liner docstrings, please keep the closing """ on the same line.

So, for example, this -

"""Receives position and player type ('X' or 'O').
Returns modified board if position was valid.
Asks to player try a different position otherwise."""

Should actually be written as -

"""Receives position and player type ('X' or 'O').
Returns modified board if position was valid.
Asks to player try a different position otherwise.
"""

Also, since you have descriptively named functions, you don't need those unnecessary comments explaining what your function does. For example, this does not need a comment -

def printing_board(self):
    """Prints the board."""
    self.board.print_board()

We know you're printing the board; it says in the function itself - def printing_board(self).

Also, good use of the if '__name__' == __'main__': guard. Most people don't even attempt to use it.


Note that the trailing \ solutions are not recommended by PEP 8. One reason is that if space is added by mistake after a \ it might not show in your editor, and the code becomes syntactically incorrect.

The PEP changed at https://hg.python.org/peps/rev/7a48207aaab6 to explicitly discourage backslashes.

The preferred way of wrapping long lines is by using Python's implied line continuation inside parentheses, brackets, and braces. Long lines can be broken over multiple lines by wrapping expressions in parentheses. These should be used in preference to using a backslash for line continuation.

Another thing is that, here (for example) -

if self.board["TL"] == player.type and self.board["TM"] == player.type and self.board["TR"] == player.type or \
self.board["ML"] == player.type and self.board["MM"] == player.type and self.board["MR"] == player.type or \
self.board["BL"] == player.type and self.board["BM"] == player.type and self.board["BR"] == player.type or \
self.board["TL"] == player.type and self.board["ML"] == player.type and self.board["BL"] == player.type or \
self.board["TM"] == player.type and self.board["MM"] == player.type and self.board["BM"] == player.type or \
self.board["TR"] == player.type and self.board["MR"] == player.type and self.board["BR"] == player.type or \
self.board["TL"] == player.type and self.board["MM"] == player.type and self.board["BR"] == player.type or \
self.board["BL"] == player.type and self.board["MM"] == player.type and self.board["TR"] == player.type:

the lines are too long. According to PEP 8 -

Limit all lines to a maximum of 79 characters.

Therefore, these statements could alternatively be written as -

def is_winner(self, player):
    player_type = player.type
    runs = [
        # horizontal
        ["TL", "TM", "TR"],
        ["ML", "MM", "MR"],
        ["BL", "BM", "BR"],
        # vertical
        ["TL", "ML", "BL"],
        ["TM", "MM", "BM"],
        ["TR", "MR", "BR"],
        # diagonal
        ["TL", "MM", "BR"],
        ["BL", "MM", "TR"]
    ]
    for a, b, c in runs:
        if self.board[a] == self.board[b] == self.board[c] == player_type:
            return True
    return False

Overall, in terms of code readability and style, this is what you need to improve. You should make your code more PEP 8 compliant.

Hope this helps!

Justin
  • 2.6k
  • 3
  • 21
  • 59