5
\$\begingroup\$

I'm a newbie in Tkinter and I've managed to make the "Lights Off" puzzle using Tkinter with Python 3.

The game starts by presenting 9 buttons arranged in a 3x3 matrix. The text of the button will be either 0 (which denotes that the light is off) and 1 (which denotes that the light is on). Clicking on a button inverts the text in it as well as the text in the adjacent buttons.

The game is won once all the lights are off, i.e, all the buttons have 0 as their text.

Right now, the program does nothing when the game is won. I haven't made that part yet.

Is there any room for improvement?

from tkinter import *
from functools import partial
from random import randint

def button_text_changer(val, buttons):
    '''Changes text of buttons'''

    buttons[val].config(text = str(int(buttons[val].config('text')[-1]) ^ 1)) # Change the text of the button pressed to 0 if it was 1 and 1 if it was 0

    # Change text of adjacent buttons
    if val == 0:
        buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
        buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
    if val == 1:
        buttons[0].config(text = str(int(buttons[0].config('text')[-1]) ^ 1))
        buttons[2].config(text = str(int(buttons[2].config('text')[-1]) ^ 1))
        buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
    if val == 2:
        buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
    if val == 3:
        buttons[0].config(text = str(int(buttons[0].config('text')[-1]) ^ 1))
        buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[6].config(text = str(int(buttons[6].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))
    if val == 4:
        buttons[0].config(text = str(int(buttons[0].config('text')[-1]) ^ 1))
        buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
        buttons[2].config(text = str(int(buttons[2].config('text')[-1]) ^ 1))
        buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
        buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
        buttons[6].config(text = str(int(buttons[6].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))
        buttons[8].config(text = str(int(buttons[8].config('text')[-1]) ^ 1))
    if val == 5:
        buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
        buttons[2].config(text = str(int(buttons[2].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))
        buttons[8].config(text = str(int(buttons[8].config('text')[-1]) ^ 1))
    if val == 6:
        buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))
    if val == 7:
        buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
        buttons[6].config(text = str(int(buttons[6].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))
    if val == 8:
        buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
        buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
        buttons[7].config(text = str(int(buttons[7].config('text')[-1]) ^ 1))

def get_rand_list():
    '''Returns a list of size 9 with zeros and ones which are randomly chosen'''

    rand_list = []

    for i in range(9):
        if randint(0, 1) == 0:
            rand_list.append(0)
        else:
            rand_list.append(1)

    # Checking if all lights are off
    all_lights_off = True
    for i in range(9):
        if rand_list[i] == 1:
            all_lights_off = False

    if all_lights_off:
        rand_list = get_rand_list() # Generate random list once again
    else:
        return rand_list

def add_Buttons(root, rand_list):
    '''Add buttons to root(first argument), with their text corresponding to the values in the list provided(second argument)'''

    row = 0
    buttons = []

    for i in range(9):

        if i != 0 and i % 3 == 0:
            row += 1

        button = Button(root, text = rand_list[i], font = ('arial', 25, 'bold'))
        button.grid(row = row, column = i % 3)

        buttons.append(button)

    for i in range(9):
        buttons[i].config(command = partial(button_text_changer, i, buttons))

def start_game():
    '''Starts the Lights Off game'''

    root = Tk()

    add_Buttons(root, get_rand_list())

    root.mainloop()


if __name__ == '__main__':
    start_game()
\$\endgroup\$

1 Answer 1

7
\$\begingroup\$

Don't repeat yourself

This is really a bit excessive repetition:

if val == 0:
    buttons[1].config(text = str(int(buttons[1].config('text')[-1]) ^ 1))
    buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
    buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
if val == 1:
    buttons[0].config(text = str(int(buttons[0].config('text')[-1]) ^ 1))
    buttons[2].config(text = str(int(buttons[2].config('text')[-1]) ^ 1))
    buttons[3].config(text = str(int(buttons[3].config('text')[-1]) ^ 1))
    buttons[4].config(text = str(int(buttons[4].config('text')[-1]) ^ 1))
    buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1))
# ...

A helper function can make this a lot easier and safer, for example:

def update_buttons(*idx_list):
    for index in idx_list:
        buttons[index].config(text = str(int(buttons[index].config('text')[-1]) ^ 1))

if val == 0:
    update_buttons(1, 3, 4)
elif val == 1:
    update_buttons(0, 2, 3, 4, 5)
# ...

Notice also that I changed the mutually exclusive if-if to if-elif.

Simplify

This can be written simpler:

    if randint(0, 1) == 0:
        rand_list.append(0)
    else:
        rand_list.append(1)

Like this:

    rand_list.append(randint(0, 1))

Use list comprehensions

This loop can be rewritten with a list comprehension:

for i in range(9):
    if randint(0, 1) == 0:
        rand_list.append(0)
    else:
        rand_list.append(1)

Like this:

rand_list = [randint(0, 1) for _ in range(9)]

Issues with iteration

In this piece, notice that after all_lights_off is set to False, there's no need to iterate further, you can break.

all_lights_off = True
for i in range(9):
    if rand_list[i] == 1:
        all_lights_off = False

But even better, the entire loop can be replaced with this:

all_lights_off = not any(rand_list)

any returns true if any element in the list is truthy. In this example, it returns when there is a 1 in the list anywhere. So, not any(rand_list) will only be True when all values are 0, because 0 is falsy.

Dead code

The assignment rand_list = ... in the if branch is dead code, because this is the last statement in the function, so the value is assigned but it will be never used. Also note that when all_lights_off is True, the function will return None, instead of a random list.

if all_lights_off:
    rand_list = get_rand_list() # Generate random list once again
else:
    return rand_list
\$\endgroup\$
2
  • \$\begingroup\$ Awesome answer. Thanks. BTW, could you change buttons[5].config(text = str(int(buttons[1].config('text')[-1]) ^ 1)) to buttons[5].config(text = str(int(buttons[5].config('text')[-1]) ^ 1)) in your answer? \$\endgroup\$
    – Spikatrix
    Sep 3, 2015 at 14:55
  • \$\begingroup\$ There you go :-) \$\endgroup\$
    – janos
    Sep 3, 2015 at 14:58

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.