Return to Revisions

#Style

Aside of @syb0rg's answer, there are some comments in the text intended to describe the method that follows. These should be docstrings instead.

You also happen to have magic numbers and sequences that would be better as constants, or at least class-level attributes:

class Simon:
    IDLE = ("red", "blue", "green", "yellow")
    TINTED = ("#ff4d4d", "#4d4dff", "#4dff4d", "#ffff4d")

    FLASH_ON = 1000
    FLASH_OFF = 250

    def __init__(self):
        ...

#Reinventing the wheel

Instead of drawing rectangles on a canvas and binding events to them, tkinter provides the Button widget which allows a lot out-of-the-box: changing the color is done with button.config(background='green'), binding an action is done with the command attribute; and you can also disable a button so user can't interact when the sequence is being displayed.

The only thing to take care of is retrieving the right button into check_choice: you'll need to add some kind of index parameter. But the command keyword used when building a button expects a no-arguments callable. You can overcome that by using helper methods that call check_choice with a fixed index, using a lambda, or using functools.partial.

#Flashing the tiles

I find the way you handle showing the sequence to the player somewhat uncommon. Having flash being called twice (on + off) between two calls of show_sequence feels weird. It would be more straightforward to have show_sequence initiate stuff and then a flash_on and a flash_off method that are calling each other using self.master.after (and using the self.FLASH_ON and self.FLASH_OFF constants).

#Iterating over the sequence

Not that your way of doing is that bad considering the constraints, but I just want to show an alternative approach: using iter and next. Instead of storing the current index, you store an iterator to you sequence using iter and the current button that should be pushed. After a successful button push, you update the current value using next. And if StopIteration is raised, then it means the player reached the end of the sequence.

This method can be used both when waiting for user input and when flashing the buttons.

#Nitpicks

I find that flashing a tile for a whole second is a little bit slow. I personally would have used 600 or 750 ms.

I also find that the tinted colors are too close to the idle ones (except for green). You should use colors that are more distinct so the player can better see which one is next in the sequence.

Lastly, I feel that running self.master.mainloop() or even having the root created outside the class does not allow for much customization, I would keep things internal and provide a method to launch the game (that will run self.master.mainloop().

#Proposed improvements

import tkinter as tk
import random
from functools import partial


class Simon:
    IDLE = ('red', 'blue', 'green', 'yellow')
    TINTED = ('#ff4d4d', '#4d4dff', '#4dff4d', '#ffff4d')

    FLASH_ON = 750  #ms
    FLASH_OFF = 250  #ms

    def __init__(self, title='Simon Memory Game'):
        self.master = tk.Tk()
        self.master.title(title)
        self.master.resizable(False, False)
        self.title = title
        self.buttons = [
            tk.Button(
                self.master,
                height=15,
                width=25,
                background=c,
                activebackground=c,  # remove this line if you want the player to see which button is hovered
                command=partial(self.push, i))
            for i, c in enumerate(self.IDLE)]
        for i, button in enumerate(self.buttons):
            button.grid({'column': i % 2, 'row': i // 2})

    def reset(self):
        self.sequence = []
        self.new_color()

    def push(self, index):
        if index == self.current:
            try:
                self.current = next(self.iterator)
            except StopIteration:
                self.master.title('{} - Score: {}'
                                  .format(self.title, len(self.sequence)))
                self.new_color()
        else:
            self.master.title('{} - Game Over! | Final Score: {}'
                              .format(self.title, len(self.sequence)))
            self.reset()

    def new_color(self):
        for button in self.buttons:
            button.config(state=tk.DISABLED)
        color = random.randrange(0, len(self.buttons))
        self.sequence.append(color)
        self.iterator = iter(self.sequence)
        self.show_tile()

    def show_tile(self):
        try:
            id = next(self.iterator)
        except StopIteration:
            # No more tiles to show, start waiting for user input
            self.iterator = iter(self.sequence)
            self.current = next(self.iterator)
            for button in self.buttons:
                button.config(state=tk.NORMAL)
        else:
            self.buttons[id].config(background=self.TINTED[id])
            self.master.after(self.FLASH_ON, self.hide_tile)

    def hide_tile(self):
        for button, color in zip(self.buttons, self.IDLE):
            button.config(background=color)
        self.master.after(self.FLASH_OFF, self.show_tile)

    def run(self):
        self.reset()
        self.master.mainloop()


if __name__ == '__main__':
    game = Simon()
    game.run()