I'm not an experienced user of either tkinter not turtle graphics, so I can't offer specific improvements with regards to those. I can offer some general Python advice though.
I like the Label class here. It was a good choice.
def __init__(self, coordinates=[0, 0], screen=screen):
This line has the potential to create a nasty and hard to find bug. The issue is with coordinates=[0,0]. I'll try to highlight why with an example.
class A():
def __init__(self, coords=[0, 0]):
self.coords = coords
apple = A()
banana = A()
print(apple.coords, banana.coords) # [0, 0] [0, 0]
apple.coords[0] = 5
print(apple.coords, banana.coords) # [5, 0] [5, 0]
As you can see from the example, somehow two different instances are sharing the coords between them. The cause of this is that the list [0, 0] is the same list each time a new instance is made. See mutable default args for more info.
I would fix it like this
class Label(turtle.Turtle):
def __init__(self, coordinates=None, screen=screen):
...
if coordinates is None:
self.coordinates = [0, 0]
else:
self.coordinates = coordinates
Then you wont hit this problem again until you start changing the coordinates of a Label that you create with the same list. Be careful with lists!
# Bad, has the same problem
coords = [0, 0]
one, two = Label(coords), Label(coords)
one.coordinates[0] = 5
print(one.coordinates, two.coordinates) # [5, 0] [5, 0]
# Good, doesn't have the problem
one, two = Label([0, 0]), Label([0, 0])
one.coordinates[0] = 5
print(one.coordinates, two.coordinates) # [5, 0] [0, 0]
def clear_labels():
global label1, label2, label3
label1.clear()
label2.clear()
label3.clear()
I would make a (global) list of labels and loop through them all. This means you can add more labels if you need to without changing much code.
all_labels = [label1, label2, label3]
def clear_labels():
for label in all_labels:
label.clear()
def show_labels(vertices):
global label1, label2, label3
label1.show(vertices[0])
label2.show(vertices[1])
label3.show(vertices[2])
(Small aside, this function and clear_labels are defined twice) I would once again loop over a global list of labels. You loop over two lists at the same time with zip, for example
for number, letter in zip([1, 2, 3], ['A', 'B', 'C']):
print(number, letter)
# 1 A
# 2 B
# 3 C
The code would look like this
def show_labels(vertices):
for label, vertex in zip(all_labels, vertices):
label.show(vertex)
def reset():
global vertices, triangle
vertices = ["A", "B", "C"]
show_labels(vertices)
Don't set a global vertices here. Pass it in if it is needed, or construct it on the fly.
def rotate_clockwise():
....
temp = vertices[-1]
for i in range(len(vertices) - 1, 0, -1):
vertices[i] = vertices[i - 1]
vertices[0] = temp
There is a much nicer way to rotate lists using slicing. It is good in that it also works for the immutable tuple as well. See this answer on stackoverflow for the source.
clear_labels()
triangle.right(120)
show_labels(vertices)
This looks like it should be part of update_rotation, especially since rotate_anticlockwise calls update_rotation.
In this example I'm taking a few liberties such as making one function do all the work, but I think it makes the example easier to digest. I don't know if I've put in all the globals I need to.
def reflect_A():
global vertices
b_pos = vertices.index("B")
c_pos = vertices.index("C")
vertices[b_pos], vertices[c_pos] = (
vertices[c_pos],
vertices[b_pos],
)
update_reflection()
def reflect_B():
...
a_pos = vertices.index("A")
c_pos = vertices.index("C")
...
Here you have a large amount of repeated code. Since only the indices change between each function, I would recommend making a new function that takes the indices to swap, and does all the work (including update_reflection).
def reflect(ind1, ind2):
"""Reflect by swapping the vertices at ind1 and ind2."""
global vertices
vertices[ind1], vertices[ind2] = vertices[ind2], vertices[ind1]
# inlined update_reflection
global triangle
clear_labels()
show_labels(vertices)
Then you can call reflect in each function. If you want to be fancy you can define the functions with partial. Partial turns a function into another function with some parameters set. Here is an example with partial.
def print_hello():
print('hello')
print_hello() # hello
from functools import partial
print_hello = partial(print, 'hello')
print_hello() # hello
And an example of defining reflect_*
reflect_A = partial(reflect, vertices.index("A"), vertices.index("B"))
reflect_B = partial(reflect, vertices.index("A"), vertices.index("C"))
reflect_C = partial(reflect, vertices.index("B"), vertices.index("C"))
And finally here are some nitpicks and/or food for thought.
- You use two different fonts, why?
- Can you change each function with a global to accept the parameter as an argument instead? Ideally every time you call a function with some set of parameters it should do exactly the same thing. For example,
f([], 'a') -> ['a']
is good,f('a') -> None
but some global variable gets updated is bad, as it tends to make code harder to {update,maintain,understand,reason about,debug,share without other people getting mad at you}. - Can you remove the turtle and do everything with tkinter? Can you remove tkinter and do everything in turtle graphics?
- Sometimes somebody wants to change something and does not want to edit code. Things like the colour of the background, or the title of the window, or the text when you hover over a button. Can you make a separate config file that has the full colour-scheme?
- What happens if you change the vertices' names from ['A', 'B', 'C'] to ['X', 'Y', 'Z']? How much of the code breaks? Should it break? Does it change everywhere it needs to?