Symmetries of Triangle Python Turtle

Question

I made a program to explore the symmetries of an equilateral triangle using Python and Turtle Graphics. It's formatted using Black Playground.

As usual, my goal is to be writing "code that doesn't suck." What do I need to to to achieve that goal is this instance please?

I'm guessing some repetition could be avoided, and I know globals are evil, but are they as evil as passing parameters to a tkinter callback?

Would the code benefit from refactoring to be entirely OOP, or would that be overkill for something so relatively simple?

import turtle
import tkinter as tk

screen = turtle.Screen()


class Label(turtle.Turtle):
    def __init__(self, coordinates=[0, 0], screen=screen):
        turtle.Turtle.__init__(self)
        self.text = ""
        self.color("white")
        self.coordinates = coordinates
        self.hideturtle()
        self.penup()
        self.screen = screen

    def show(self, message, alignment="center", size=18):
        self.screen.tracer(0)
        self.clear()
        self.goto(self.coordinates)
        self.write(
            message, font=("Arial", size), align=alignment
        )
        self.screen.tracer(1)


def show_labels(vertices):
    global label1, label2, label3
    label1.show(vertices[0])
    label2.show(vertices[1])
    label3.show(vertices[2])


def clear_labels():
    global label1, label2, label3
    label1.clear()
    label2.clear()
    label3.clear()


def reset():
    global vertices, triangle
    vertices = ["A", "B", "C"]
    show_labels(vertices)


def rotate_clockwise():
    global vertices, triangle
    temp = vertices[-1]
    for i in range(len(vertices) - 1, 0, -1):
        vertices[i] = vertices[i - 1]
    vertices[0] = temp

    clear_labels()
    triangle.right(120)
    show_labels(vertices)


def rotate_anticlockwise():
    global vertices, triangle
    temp = vertices[0]
    for i in range(len(vertices) - 1):
        vertices[i] = vertices[i + 1]
    vertices[-1] = temp

    update_rotation()


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():
    global vertices
    a_pos = vertices.index("A")
    c_pos = vertices.index("C")
    vertices[a_pos], vertices[c_pos] = (
        vertices[c_pos],
        vertices[a_pos],
    )

    update_reflection()


def reflect_C():
    global vertices
    a_pos = vertices.index("A")
    b_pos = vertices.index("B")
    vertices[a_pos], vertices[b_pos] = (
        vertices[b_pos],
        vertices[a_pos],
    )

    update_reflection()


def update_rotation():
    global triangle
    clear_labels()
    triangle.left(120)
    show_labels(vertices)


def update_reflection():
    global triangle
    clear_labels()
    show_labels(vertices)


def show_labels(vertices):
    global label1, label2, label3
    label1.show(vertices[0])
    label2.show(vertices[1])
    label3.show(vertices[2])


def clear_labels():
    global label1, label2, label3
    label1.clear()
    label2.clear()
    label3.clear()


canvas = screen.getcanvas()
font = ("Helvetica", 12)

reset_button = tk.Button(
    canvas.master,
    font=font,
    text="Reset",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reset,
)
canvas.create_window(0, -250, window=reset_button)

rotate_clockwise_button = tk.Button(
    canvas.master,
    font=font,
    text="Rotate 120° clockwise",
    background="hotpink",
    foreground="white",
    bd=0,
    command=rotate_clockwise,
)
canvas.create_window(
    0, -150, window=rotate_clockwise_button
)

rotate_anticlockwise_button = tk.Button(
    canvas.master,
    font=font,
    text="Rotate 120° anticlockwise",
    background="hotpink",
    foreground="white",
    bd=0,
    command=rotate_anticlockwise,
)
canvas.create_window(
    0, -200, window=rotate_anticlockwise_button
)

reflect_A_button = tk.Button(
    canvas.master,
    font=font,
    text="Reflect about perp. bisector of BC",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reflect_A,
)
canvas.create_window(0, 100, window=reflect_A_button)

reflect_B_button = tk.Button(
    canvas.master,
    font=font,
    text="Reflect about perp. bisector of AC",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reflect_B,
)
canvas.create_window(0, 150, window=reflect_B_button)

reflect_C_button = tk.Button(
    canvas.master,
    font=font,
    text="Reflect about perp. bisector of AB",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reflect_C,
)
canvas.create_window(0, 200, window=reflect_C_button)

screen.setup(500, 600)
screen.title("Symmetries of an Equilateral Triangle")
screen.bgcolor("blue")

label1 = Label([-85, -55])
label2 = Label([0, 75])
label3 = Label([85, -55])

triangle = turtle.Turtle("triangle")
triangle.shapesize(120 / 20)
triangle.color("hotpink")
triangle.right(30)

vertices = ["A", "B", "C"]
show_labels(vertices)

turtle.done()

Answer 1

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.

1. You use two different fonts, why?
2. 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}.
3. Can you remove the turtle and do everything with tkinter? Can you remove tkinter and do everything in turtle graphics?
4. 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?
5. 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?

Answer 2

Starting at the bottom of the code:

You don't have an if __name__ == '__main__': block. Unless things have changed in Python 3, you really ought to have one. (This distinguishes "module setup" code that should be run on every import of this .py file, from "main program" code that should only run when the .py file is run as a program.) Arguably it's just boilerplate in this case, but I think it's good-habit-forming boilerplate.

---

font = ("Helvetica", 12)

reset_button = tk.Button(
    canvas.master,
    font=font,
    text="Reset",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reset,
)

On the other hand, this feels like bad boilerplate. Global variables are never nice, even when you promise their values are constant. Kneejerk, I would prefer to write

reset_button = tk.Button(
    canvas.master,
    font=("Helvetica", 12),
    text="Reset",
    background="hotpink",
    foreground="white",
    bd=0,
    command=reset,
)

But you'd have to duplicate that font=("Helvetica", 12) assignment in several places! Well, programming is the art of detecting and removing duplication. Let's take the duplicated code and factor it out:

def make_button(canvas, text, command):
    return tk.Button(
        canvas.master,
        font=("Helvetica", 12),
        text=text,
        background="hotpink",
        foreground="white",
        bd=0,
        command=command,
    )

reset_button = make_button(canvas, "Reset", reset)
canvas.create_window(0, -250, window=reset_button)

rotate_clockwise_button = make_button(canvas, "Rotate 120° clockwise", rotate_clockwise)
canvas.create_window(
    0, -150, window=rotate_clockwise_button
)

rotate_anticlockwise_button = make_button(canvas, "Rotate 120° anticlockwise", rotate_anticlockwise)
canvas.create_window(
    0, -200, window=rotate_anticlockwise_button
)

reflect_A_button = make_button("Reflect about perp. bisector of BC", reflect_A)
canvas.create_window(0, 100, window=reflect_A_button)

[...]

Now we see some more repetition. We also see a bunch of one-use global variables, such as reset_button, which don't need to be global (or exist at all, really). We also see one surprising asymmetry: you wrote the call to canvas.create_window as a three-liner in one place, whereas it's a one-liner in every other place.

So let's factor out the repetition and eliminate the asymmetry:

def create_button(canvas, x, y, text, command):
    canvas.create_window(x, y, window=make_button(canvas, text, command))

canvas = screen.getcanvas()
create_button(canvas, 0, -250, "Reset", reset)
create_button(canvas, 0, -150, "Rotate 120° clockwise", rotate_clockwise)
create_button(canvas, 0, -200, "Rotate 120° anticlockwise", rotate_anticlockwise)
create_button(canvas, 0, 100, "Reflect about perp. bisector of BC", reflect_A)
create_button(canvas, 0, 150, "Reflect about perp. bisector of AC", reflect_B)
create_button(canvas, 0, 200, "Reflect about perp. bisector of AB", reflect_C)

What took you 72 lines in the original code, now takes 21 lines.

---

I suggest applying this kind of redundancy-removal everywhere you can in your code, and then re-posting it as a new question. There are more interesting possibilities to discuss, re your interest in triangle symmetries. For example, maybe instead of hard-coding ideas like "the altitude through A hits the midpoint of BC; the altitude through B hits the midpoint of AC; the altitude through C hits the midpoint of AB," maybe you could encode the general principle that "the altitude through (x) hits the midpoint of (the two labeled points that aren't x)." You might approach that in roughly the same way that I approached the idea of "how to create a button with text (x) and command (y)."