patternpythonMinor
"Lights Off" puzzle in tkinter
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]
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:
A helper function can make this a lot easier and safer, for example:
Notice also that I changed the mutually exclusive
Simplify
This can be written simpler:
Like this:
Use list comprehensions
This loop can be rewritten with a list comprehension:
Like this:
Issues with iteration
In this piece,
notice that after
there's no need to iterate further, you can
But even better, the entire loop can be replaced with this:
In this example, it returns when there is a 1 in the list anywhere.
So,
because 0 is falsy.
Dead code
The assignment
because this is the last statement in the function,
so the value is assigned but it will be never used.
Also note that when
the function will return
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 = FalseBut 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_listCode 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.