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()
2 Answers 2
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] = []
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()
```