7
\$\begingroup\$

I am working through Programming Fundamentals with Python on Udacity. The assignment was to create a recursive triangle. I am happy with the output, but I am hoping for feedback on the actual construction of the program. Could I have done this in a more simple and concise way?

import turtle

def draw_polygon(a_turtle, length, sides):
    counter = 0
    while (counter < sides):
        a_turtle.forward(length)
        a_turtle.right(360 / sides)
        counter = counter + 1

def draw_triangle(a_turtle, length):
    draw_polygon(a_turtle, length, 3)

def draw_fractal_triangle(a_turtle, length, depth):
    if (depth == 1):
        draw_triangle(a_turtle, length)
    else:
        for i in range(1,4):
            draw_fractal_triangle(a_turtle, length/2, depth-1)
            a_turtle.forward(length)
            a_turtle.right(120)

def draw():
    window = turtle.Screen()
    window.bgcolor("black")

    brent = turtle.Turtle()
    brent.shape("turtle")
    brent.color("yellow")

    length = 200
    brent.backward(length/2)
    draw_fractal_triangle(brent, length, 5)
    window.exitonclick()

draw()


```
\$\endgroup\$
4
\$\begingroup\$

Nice. I only have three comments.

  1. In draw_polygon() use a for loop rather than an explicit counter:

    def draw_polygon(a_turtle, length, sides):
        for counter in range(sides):
            a_turtle.forward(length)
            a_turtle.right(360 / sides)
    
  2. Add comments and/or docstrings. These will help you when you look at the code in the future.

    For these kinds of tasks I usually add a top level docstring with a description of the task. Include a url to the problem if applicable. For example at the top of the file:

    '''Recursive implementation of Sierpinski Triangle
    
    Assignment from Programming Fundamentals with Python on Udacity.
    '''
    

    For any function/method etc. that has an algorithm that isn't immediately obvious from the code, add a comment or docstring explaining what/how it's doing it. For example, I would add a docstring to draw_recursive_triangle() to explain what the function is doing, any assumptions (does it matter which way the turtle is pointing?, are there min or max limits on length? are the triangles always equilateral?, etc.).

  3. The functions might be useful in another program/assignment. Rather than rewriting them, you could import this file as a library if you use a if __name__ == "__main__": guard like so:

    if __name__ == "__main__":
        draw()
    

    That way it runs the program if you execute the file, but not if you import it as a library

\$\endgroup\$
3
\$\begingroup\$

I'm a python beginner,so at the moment I can only see that you can slightly improve your code using range in the function draw_polygon, so instead of

    counter = 0
    while (counter < sides):
        a_turtle.forward(length)
        a_turtle.right(360 / sides)
        counter = counter + 1

You can rewrite it in this way avoiding multiple divisions inside the loop and manual incrementing of variable counter:

angle = 360 / sides
for counter in range(sides):
    a_turtle.forward(length)
    a_turtle.right(angle)
\$\endgroup\$
2
\$\begingroup\$

Good luck in your learning, very nice start, i was much poorer in my begining! Viewing your code i suggest a structural change besides what @RootTwo and @dariosicily told.

The Spirit of Functions

Functions should be called with relevent paramenters. Currently when calling draw_triangle we need to pass the a_turtle parameter

draw_triangle(a_turtle, length)

but it would be nicer if we could call it by

draw_triangle(length)

directly.

Nicer Functions With Global Variables

By defining and using global variables, the above is achievable.

brent = turtle.Turtle()
window = turtle.Screen()

Then modifying functions using the global keyword. The global keyword allows you to use global variables within functions

def draw_polygon(length, sides):
    global brent
    for i in range(sides):
        brent.forward(length)
        brent.right(360 / sides)

The draw function

def draw():
    global brent, window
    window.bgcolor("black")
    brent.shape("turtle")
    ...

Then no need to each time add the a_turtle parameter

The Class Approach

But, global variables might be a sign you need an OOP approach

This is an OOP approched by changing the above

class Sierpinski:
    def __init__(self):
        self.brent = turtle.Turtle()
        self.window = turtle.Screen()

    def draw_polygon(self, length, sides):
        brent = self.brent
        for i in range(sides):
            brent.forward(length)
            brent.right(360 / sides)

    def draw_triangle(self, length):
        self.draw_polygon(length, 3)

    def draw_fractal_triangle(self, length, depth):
        brent = self.brent
        if (depth == 1):
            self.draw_triangle(length)
        else:
            for i in range(1, 4):
                self.draw_fractal_triangle(length/2, depth-1)
                brent.forward(length)
                brent.right(120)

    def draw(self):
        brent = self.brent
        window = self.window

        window.bgcolor("black")
        brent.shape("turtle")
        brent.color("yellow")
        length = 200
        brent.backward(length/2)
        self.draw_fractal_triangle(length, 5)
        window.exitonclick()

then to draw,

s = Sierpinski()
s.draw()

you could also implement

    def draw_polygon(self, length, sides):
        brent = self.brent
        for i in range(sides):
            brent.forward(length)
            brent.right(360 / sides)

as

    def draw_polygon(self, length, sides):
        for i in range(sides):
            self.brent.forward(length)
            self.brent.right(360 / sides)

More control

Specifying the length and depth in the constructor might allow you to have more control, by changing values at one place, you modify it all

class Sierpinski:
    def __init__(self):
        self.brent = turtle.Turtle()
        self.window = turtle.Screen()
        self.length = 200
        self.depth = 5

modifying in draw

    def draw(self):
        brent = self.brent
        window = self.window
        length = self.length
        depth = self.depth

        window.bgcolor("black")
        brent.shape("turtle")
        brent.color("yellow")

        brent.backward(length/2)
        self.draw_fractal_triangle(length, depth)
        window.exitonclick()

Parameters

You can let users pass their own value by passing the values as parameters

class Sierpinski:
    def __init__(self, length, depth):
        self.brent = turtle.Turtle()
        self.window = turtle.Screen()
        self.length = length
        self.depth = depth

usage:

s = Sierpinski(200, 5)
s.draw()

One last improvement could be adding parameters to draw instead of the class itself.

Stack Overflow advice

When writing codes,

leave a line after the last ``` symbol to prevent this

code bug

\$\endgroup\$
  • 3
    \$\begingroup\$ i.imgur.com/HZTbLUB.jpg Almost downvoted for "Functions should be called with relevent paramenters" and then implying that a_turtle is somehow not a relevant parameter to a function that needs a turtle. "Use global vars" is never good advice. You turned it around in the second half, but I don't think you really needed to go via "global variables" to get where you were going. Great answer otherwise. \$\endgroup\$ – Quuxplusone Aug 10 at 15:03
  • 2
    \$\begingroup\$ The global keyword is unnecessary here. You only need it when you assign to the variable (= or augmented assignment). But here, you only read the variables. \$\endgroup\$ – Wombatz Aug 10 at 16:45
  • 2
    \$\begingroup\$ Please pass in variables instead of using global. global is almost never necessary and breaks encapsulation, rendering the program brittle and error-prone. Functions should have no side effects and should be reducing cognitive load for the programmer, not increasing it. The numerous assignments in draw, i.e. brent = self.brent ... window = self.window are unnecessary and obsfuscative of variable ownership. The class refactor and parameters sections are helpful and on-point, though. \$\endgroup\$ – ggorlen Aug 10 at 17:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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