Skip to main content
replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link

On comments

# On button pressed handler
def btn_pressed(self, button):

I think you could rename the function to on_btn_pressed and just remove the comment, as it doesn't add any information that isn't already apparent from the function name.

There are several other comments that doesn't add anything useful, like

# Initializes players
def init_players(self):

and

# Checks winner after every move...
def check_winner(self):

and

self.make_random_move(board) # Do a random move

The comments describing the build and on_start functions could do with some more explanation, as it's not apparent to me how these are used (I'm assuming they are used by kivy.app.App?).

if len(button.text.strip()) < 1: # If the button has no mark, stripping spaces...

I would extract the above check into a function, like this:

if is_button_unmarked(button):
    free_buttons.append(button)
...

def is_button_unmarked(self, button):
    return len(button.text.strip()) < 1

thus eliminating the need for a comment.

You should strive to make the code self-documenting, and use comments when you fail to do that. I recommend reading rolfl's excellent answerexcellent answer to a codereview question about comments.

Constructor vs method injection

I would inject the game board into the Ai constructor

self.bot = Ai(self.choices[rand_choice], board)

and set it as you do with the choice

class Ai:
    def __init__(self, choice, board):
        self.choice = choice
        self.board = board

This way you can do

self.bot.make_move()

instead of

self.bot.make_move(self.board)

which is easier to understand. As the board is used by all the functions in Ai, there's currently no good reason not to inject it in the constructor.

Reuse

Your popup_message function lets you pass any message, but the title is hardcoded. You should send the title in as an argument as well, to make the popup_message function useable by more than welcome messages (maybe you'd want to use it after deciding a winner in check_winner?).

On comments

# On button pressed handler
def btn_pressed(self, button):

I think you could rename the function to on_btn_pressed and just remove the comment, as it doesn't add any information that isn't already apparent from the function name.

There are several other comments that doesn't add anything useful, like

# Initializes players
def init_players(self):

and

# Checks winner after every move...
def check_winner(self):

and

self.make_random_move(board) # Do a random move

The comments describing the build and on_start functions could do with some more explanation, as it's not apparent to me how these are used (I'm assuming they are used by kivy.app.App?).

if len(button.text.strip()) < 1: # If the button has no mark, stripping spaces...

I would extract the above check into a function, like this:

if is_button_unmarked(button):
    free_buttons.append(button)
...

def is_button_unmarked(self, button):
    return len(button.text.strip()) < 1

thus eliminating the need for a comment.

You should strive to make the code self-documenting, and use comments when you fail to do that. I recommend reading rolfl's excellent answer to a codereview question about comments.

Constructor vs method injection

I would inject the game board into the Ai constructor

self.bot = Ai(self.choices[rand_choice], board)

and set it as you do with the choice

class Ai:
    def __init__(self, choice, board):
        self.choice = choice
        self.board = board

This way you can do

self.bot.make_move()

instead of

self.bot.make_move(self.board)

which is easier to understand. As the board is used by all the functions in Ai, there's currently no good reason not to inject it in the constructor.

Reuse

Your popup_message function lets you pass any message, but the title is hardcoded. You should send the title in as an argument as well, to make the popup_message function useable by more than welcome messages (maybe you'd want to use it after deciding a winner in check_winner?).

On comments

# On button pressed handler
def btn_pressed(self, button):

I think you could rename the function to on_btn_pressed and just remove the comment, as it doesn't add any information that isn't already apparent from the function name.

There are several other comments that doesn't add anything useful, like

# Initializes players
def init_players(self):

and

# Checks winner after every move...
def check_winner(self):

and

self.make_random_move(board) # Do a random move

The comments describing the build and on_start functions could do with some more explanation, as it's not apparent to me how these are used (I'm assuming they are used by kivy.app.App?).

if len(button.text.strip()) < 1: # If the button has no mark, stripping spaces...

I would extract the above check into a function, like this:

if is_button_unmarked(button):
    free_buttons.append(button)
...

def is_button_unmarked(self, button):
    return len(button.text.strip()) < 1

thus eliminating the need for a comment.

You should strive to make the code self-documenting, and use comments when you fail to do that. I recommend reading rolfl's excellent answer to a codereview question about comments.

Constructor vs method injection

I would inject the game board into the Ai constructor

self.bot = Ai(self.choices[rand_choice], board)

and set it as you do with the choice

class Ai:
    def __init__(self, choice, board):
        self.choice = choice
        self.board = board

This way you can do

self.bot.make_move()

instead of

self.bot.make_move(self.board)

which is easier to understand. As the board is used by all the functions in Ai, there's currently no good reason not to inject it in the constructor.

Reuse

Your popup_message function lets you pass any message, but the title is hardcoded. You should send the title in as an argument as well, to make the popup_message function useable by more than welcome messages (maybe you'd want to use it after deciding a winner in check_winner?).

Source Link
AleksG
  • 256
  • 1
  • 6

On comments

# On button pressed handler
def btn_pressed(self, button):

I think you could rename the function to on_btn_pressed and just remove the comment, as it doesn't add any information that isn't already apparent from the function name.

There are several other comments that doesn't add anything useful, like

# Initializes players
def init_players(self):

and

# Checks winner after every move...
def check_winner(self):

and

self.make_random_move(board) # Do a random move

The comments describing the build and on_start functions could do with some more explanation, as it's not apparent to me how these are used (I'm assuming they are used by kivy.app.App?).

if len(button.text.strip()) < 1: # If the button has no mark, stripping spaces...

I would extract the above check into a function, like this:

if is_button_unmarked(button):
    free_buttons.append(button)
...

def is_button_unmarked(self, button):
    return len(button.text.strip()) < 1

thus eliminating the need for a comment.

You should strive to make the code self-documenting, and use comments when you fail to do that. I recommend reading rolfl's excellent answer to a codereview question about comments.

Constructor vs method injection

I would inject the game board into the Ai constructor

self.bot = Ai(self.choices[rand_choice], board)

and set it as you do with the choice

class Ai:
    def __init__(self, choice, board):
        self.choice = choice
        self.board = board

This way you can do

self.bot.make_move()

instead of

self.bot.make_move(self.board)

which is easier to understand. As the board is used by all the functions in Ai, there's currently no good reason not to inject it in the constructor.

Reuse

Your popup_message function lets you pass any message, but the title is hardcoded. You should send the title in as an argument as well, to make the popup_message function useable by more than welcome messages (maybe you'd want to use it after deciding a winner in check_winner?).