HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

"Lights Off" puzzle in tkinter

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lightspuzzleofftkinter

Problem

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]

Solution

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

Code Snippets

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))
# ...
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)
# ...
if randint(0, 1) == 0:
        rand_list.append(0)
    else:
        rand_list.append(1)
rand_list.append(randint(0, 1))
for i in range(9):
    if randint(0, 1) == 0:
        rand_list.append(0)
    else:
        rand_list.append(1)

Context

StackExchange Code Review Q#102602, answer score: 7

Revisions (0)

No revisions yet.