5
\$\begingroup\$

I am a total beginner in Python and I am trying with the modules Turtle. I am currently trying to code for 3 sliders (R, G, B) for adjusting the colors of the turtle screen. However, I found parts of my codes filled with redundancies, could anyone please tell me how to trim the code to make it more compact and clear, especially in the draw_red, draw_green, draw_blue parts? Thanks in advance.

import turtle

turtle.setworldcoordinates(0, 0, 4, 255)
turtle.colormode(255)
turtle.tracer(False)

colors = ['red', 'green', 'blue']
painters = []
red_x, green_x, blue_x = 1, 2, 3
turtle.bgcolor(0,0,0)
def spawningPainters():
    global painter
    for i in range(len(colors)):
        painter = turtle.Turtle()
        painters.append(painter)
        painter.up()
        painter.speed(0)
        painter.setheading(90)
        painter.color(colors[i])
        painter.pencolor('black')
        painter.shape('turtle')
        painter.shapesize(5, 5, 5)
        xPos = i + 1
        painter.goto(xPos, 0)

def assign_painters():
    global redPainter, greenPainter, bluePainter
    redPainter = painters[0]
    greenPainter = painters[1]
    bluePainter = painters[2]

def draw_red(x, y):
    redPainter.ondrag(None)
    x = red_x
    redPainter.goto(x, y)
    redPainter.ondrag(draw_red)
    update_screen_color()

def draw_green(x, y):
    greenPainter.ondrag(None)
    x = green_x
    greenPainter.goto(x, y)
    greenPainter.ondrag(draw_green)
    update_screen_color()

def draw_blue(x, y):
    bluePainter.ondrag(None)
    x = blue_x
    bluePainter.goto(x, y)
    bluePainter.ondrag(draw_blue)
    update_screen_color()

def update_screen_color():
    red = max(min(redPainter.ycor(), 255) , 0)
    green = max(min(greenPainter.ycor(), 255), 0)
    blue = max(min(bluePainter.ycor(), 255), 0)

    turtle.bgcolor(int(red), int(green), int(blue))

def back_to_origin():
    for painterIndex in range(len(painters)):
        painters[painterIndex].goto(painterIndex + 1, 0)

def listening_input():
    redPainter.ondrag(draw_red)
    greenPainter.ondrag(draw_green)
    bluePainter.ondrag(draw_blue)
    turtle.onkeypress(back_to_origin, 'c')

def main():
    spawningPainters()
    assign_painters()
    listening_input()

main()
turtle.listen()
turtle.tracer(True)
turtle.mainloop()
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

Review

I wasn't able to figure out a way to make the code more compact and in my opinion the code is pretty clear. However I do have some other feedback.

Use a proper main

Instead of:

turtle.setworldcoordinates(0, 0, 4, 255)
turtle.colormode(255)
turtle.tracer(False)
turtle.bgcolor(0,0,0)

...

def main():
    spawningPainters()
    assign_painters()
    listening_input()

main()
turtle.listen()
turtle.tracer(True)
turtle.mainloop()

Do:

...

if __name__ == "__main__":
    turtle.setworldcoordinates(0, 0, 4, 255)
    turtle.colormode(255)
    turtle.tracer(False)
    turtle.bgcolor(0,0,0)

    spawningPainters()
    assign_painters()
    listening_input()

    turtle.listen()
    turtle.tracer(True)
    turtle.mainloop()

For more information you can look at this post: https://stackoverflow.com/questions/419163/what-does-if-name-main-do

Prevent the turtle from going outside of the window

With the current code it is possible to drag the turtle outside of the window. If you then stop dragging you cannot get it back unless you reset the position. To prevent this from happening we can limit the boundaries of y inside of the draw functions.

def draw_blue(x, y):
    bluePainter.ondrag(None)
    x = blue_x
    y = min(y, 255)
    y = max(y, 0)
    bluePainter.goto(x, y)
    bluePainter.ondrag(draw_blue)
    update_screen_color()

By doing this the min and max calls can be removed from the update_screen_color function. However you may want to leave them as a form of defensive programming in case you add more features.

def update_screen_color():
    red = redPainter.ycor()
    green = greenPainter.ycor()
    blue = bluePainter.ycor()

    turtle.bgcolor(int(red), int(green), int(blue))

Update screen color when going back to the origin

When the position is put back to the origin the screen color is not redrawn. Unless this is intended it would be good to call the update_screen_color function at the end of the back_to_origin function.

Few tips

The global painter variable is not used anywhere outside of the spawningPainters function, so it can be removed.

Move the colors list to the spawningPainters function as it is not used anywhere else.

You might want to rename spawningPainters to spawn_painters.

Extra

Add typing to the painters list

When an empty list is declared the intellisense of your IDE may have trouble figuring out what type of elements the list will contain. By adding type annotations you can help the intellisense better understand your code. Instead of:

painters = []

Do:

from typing import List

...

painters: List[turtle.Turtle] = []
\$\endgroup\$
0
\$\begingroup\$

EDIT: The answer ended up being quite long and I don't want you to forget that you did a really nice job with this program and, especially, in recognising that there probably was a better alternative than tripling the code, one piece per colour.

This is my take on a less repetitive version of the same code. Notice that my suggestions aren't necessarily the best, and you, and I, and other people that eventually chime in, might be able to design an even better program :)

Taking care of the state

You had the right idea: you wanted a container with all the turtles so you didn't have to have a variable for each one of them, but executing that is hard because things are interconnected and you need many of your functions to be aware of many things at once.

On a first iteration I was trying to redesign your code by keeping the three painters in a dictionary that would be then passed around the several functions, but I ended up going using OOP (object-oriented programming) because this is the type of situation where it comes in handy.

We create objects that are connected and related, and let them store their own relationships and etc.

Please notice that this is not the only way to go about doing this.

Your own turtle

First, we need our own turtles that are aware of themselves, so that their .ondrag can know about their current position and etc.

import turtle

class Painter(turtle.Turtle):
    def __init__(self, turtle_app, i, colour):
        super().__init__()
        self.turtle_app = turtle_app
        self.up()
        self.speed(0)
        self.setheading(90)
        self.color(colour)
        self.pencolor('black')
        self.shape('turtle')
        self.shapesize(5, 5, 5)
        self.goto(i, 0)
        self.ondrag(self.draw)

    def draw(self, x, y):
        x = self.xcor()
        self.goto(x, y)
        self.turtle_app.update_screen_colour()

Notice that the __init__ method is the thing that initialises the turtle and does almost all of the things you had in your spawningPainters method. The things that are different are that the Painter itself defines its own draw method, and that it stores a reference to this turtle_app which is the "main controller" of your app:

class TurtleApp:
    def __init__(self):
        self.spawn_painters()

    def spawn_painters(self):
        self.painters = {}
        for i, colour in enumerate(["red", "green", "blue"], start=1):
            self.painters[colour[0]] = Painter(self, i, colour)

    def update_screen_colour(self):
            r = max(min(self.painters["r"].ycor(), 255) , 0)
            g = max(min(self.painters["g"].ycor(), 255), 0)
            b = max(min(self.painters["b"].ycor(), 255), 0)

            turtle.bgcolor(int(r), int(g), int(b))

    def reset(self):
        for painter in self.painters.values():
            painter.goto(painter.xcor(), 0)
        self.update_screen_colour()

The TurtleApp object stores the painters in a dictionary indexed by the first letter of the colour and provides the helper methods you had to reset positions, update screen colour, etc.

Using a "proper main" like another answer suggested (and which is a good suggestion!) your code ends with

if __name__ == "__main__":
    turtle.setworldcoordinates(0, 0, 4, 255)
    turtle.colormode(255)
    turtle.tracer(False)
    turtle.bgcolor(0,0,0)

    ta = TurtleApp()
    turtle.onkeypress(ta.reset, 'c')

    turtle.listen()
    turtle.tracer(True)
    turtle.mainloop()

Notice the two lines that I separated, where you create your TurtleApp and then tell turtle to call the TurtleApp.reset if the user presses the "c" key. This has been tested and seems to be working as your original one was.

Remove irrelevant variables

Try to make your code as self-sufficient as possible. You have dedicated variables to store the x position of each painter, and that is perfectly fine, but notice that as soon as you initialise it, you can always query its x position with painter.xcor(), which you know because you did the same for y with painter.ycor() when updating the background colour.

Now, if you know the x position of your painter, you can always prevent it from moving to the left or right by doing something like painter.goto(painter.xcor(), y) in the draw function, you don't need to keep the x_red, x_blue, and x_green variables with you :)

Enumerate

enumerate (which I used in the for loop to initialise the painters) is really helpful when you need to iterate over a set of values (in my case, the colours) and also know their index (which you use to set the x position in the beginning). If you don't know enumerate, you can read about it in the docs or in this tutorial. Notice that I also used the often-overlooked start optional argument to tell enumerate to start counting from 1 instead of 0.

I did one other thing:

PEP 8 naming conventions

Python usually prefers snake_case names, so that is why all my functions have capitalisation that differs from your original ones.

Final code proposal

Now everything together:

import turtle

class Painter(turtle.Turtle):
    def __init__(self, turtle_app, i, colour):
        super().__init__()
        self.turtle_app = turtle_app
        self.up()
        self.speed(0)
        self.setheading(90)
        self.color(colour)
        self.pencolor('black')
        self.shape('turtle')
        self.shapesize(5, 5, 5)
        self.goto(i, 0)
        self.ondrag(self.draw)

    def draw(self, x, y):
        x = self.xcor()
        self.goto(x, y)
        self.turtle_app.update_screen_colour()

class TurtleApp:
    def __init__(self):
        self.spawn_painters()

    def spawn_painters(self):
        self.painters = {}
        for i, colour in enumerate(["red", "green", "blue"], start=1):
            self.painters[colour[0]] = Painter(self, i, colour)

    def update_screen_colour(self):
            r = max(min(self.painters["r"].ycor(), 255) , 0)
            g = max(min(self.painters["g"].ycor(), 255), 0)
            b = max(min(self.painters["b"].ycor(), 255), 0)

            turtle.bgcolor(int(r), int(g), int(b))

    def reset(self):
        for painter in self.painters.values():
            painter.goto(painter.xcor(), 0)
        self.update_screen_colour()
        

if __name__ == "__main__":
    turtle.setworldcoordinates(0, 0, 4, 255)
    turtle.colormode(255)
    turtle.tracer(False)
    turtle.bgcolor(0,0,0)
    ta = TurtleApp()
    turtle.onkeypress(ta.reset, 'c')
    turtle.listen()
    turtle.tracer(True)
    turtle.mainloop()
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.