5
\$\begingroup\$

I made a game in python, where the objective is to catch a 'turtle'. The controls are the arrow keys.

#Catch the turtle
import turtle
import math
import random

score = 0
print ("\n" * 40)
print("Your score is:\n0")

#Title
t=turtle.Pen()
t.pencolor("Blue")
t.hideturtle()
t.penup()
t.setposition(-70,350)
t.write("Catch the turtle", font=("Verdana", 18))

#Tip
text=turtle.Pen()
t.pencolor("Red")
t.hideturtle()
t.penup()
t.setposition(-70, -350)
t.write("DON'T TOUCH THE EDGES!", font=("Verdana", 18))



#Set up screen
wn = turtle.Screen()
wn.bgcolor("lightblue")
wn.title("Catch the Turtle")

#Draw border
mypen = turtle.Turtle()
mypen.penup()
mypen.speed(10)
mypen.hideturtle()
mypen.setposition(-300,-300)
mypen.pendown()
mypen.pensize(3)
for side in range(4):
    mypen.color("yellow")
    mypen.forward(300)
    mypen.color("black")
    mypen.forward(300)
    mypen.left(90)
mypen.hideturtle()


#Create player turtle
player = turtle.Turtle()
player.color("blue")
player.shape("arrow")
player.penup()
player.speed(0)

#Create goal
goal = turtle.Turtle()
goal.color("red")
goal.shape("turtle")
goal.penup()
goal.speed(0)
goal.setposition(-100, -100)

#Set speed
speed = 1

#Define functions

def turnleft():
    player.left(30)

def turnright():
    player.right(30)

def increasespeed():
    global speed
    speed +=0.5

def decreasespeed():
    global speed
    speed -= 1

#Set keyboard binding
turtle.listen()
turtle.onkey(turnleft, "Left")
turtle.onkey(turnright, "Right")
turtle.onkey(increasespeed, "Up")
turtle.onkey(decreasespeed, "Down")



while True:
    player.forward(speed)

    #Boundary check
    if player.xcor() > 300 or player.xcor() < -300:
        print("GAME OVER")
        quit()

    if player.ycor() > 300 or player.ycor() < -300:
        print("Game OVER")
        quit()

    #Collision checking
    d= math.sqrt(math.pow(player.xcor()-goal.xcor(),2) + math.pow(player.ycor()-goal.ycor(),2))
    if d < 20 :
        goal.setposition(random.randint(-300,300), random.randint(-300, 300))
        score = score + 1
        print ("\n" * 40)
        print("Your score is")
        print (score)
\$\endgroup\$
  • 1
    \$\begingroup\$ what is the question? \$\endgroup\$ – Liam Mar 22 '17 at 13:32
  • \$\begingroup\$ This seems clear enough to me. The default question is always, does this code make my ass look fat? =) \$\endgroup\$ – Stop ongoing harm to Monica Sep 1 '17 at 9:02
1
\$\begingroup\$

Using Variables

score = 0
print ("\n" * 40)
print("Your score is:\n0")

You are setting the score to 0, then printing the value 0 as the score. What if you decided, that the player starts with 100 score points? You would have to change the value in two places. Since you already have the value in a variable, it is better to print the value of the variable. Like this: print('Your score is {0}'.format(score)) Note that I also removed the newline from the output, because Your score is 0 reads much more naturally than
Your score is:
0.


Using Functions

#Set up screen
wn = turtle.Screen()
wn.bgcolor("lightblue")
wn.title("Catch the Turtle")

This comment already reads like a function name, so pack the code inside of a function. This way you don't even need the comment, because the function name tells you all you have to know. It could look like this:

def setup_screen():
    wn = turtle.Screen()
    wn.bgcolor("lightblue")
    wn.title("Catch the Turtle")

Even better you can add parameters, to be able to easily change how the screen is setup without having to read and change the setup code:

def setup_screen(background_color, title):
    wn = turtle.Screen()
    wn.bgcolor(background_color)
    wn.title(title)

That way you achieve a good level of abstraction, where you don't have to read all of the details everytime you want to change something.


The same applies to many more blocks of code that begin with such a comment that sounds like "do this and that".


Logic and Bugs

#Title
t=turtle.Pen()
t.pencolor("Blue")
t.hideturtle()
t.penup()
t.setposition(-70,350)
t.write("Catch the turtle", font=("Verdana", 18))

#Tip
text=turtle.Pen()
t.pencolor("Red")
t.hideturtle()
t.penup()
t.setposition(-70, -350)
t.write("DON'T TOUCH THE EDGES!", font=("Verdana", 18))

In the second block, you are first initializing a variable text, but then you continue using the variable t that you used in the block before. This way you are changing values that you already set, and not changing others that you expect to be changed. I think all occurences of t in the second block should be text instead. However, if you were using functions as I explained above, this problem would not occur and you would be less likely to get this bug, which is probably caused by copy-pasting code and forgetting to change it.


#Boundary check
if player.xcor() > 300 or player.xcor() < -300:
    print("GAME OVER")
    quit()

if player.ycor() > 300 or player.ycor() < -300:
    print("Game OVER")
    quit()

This is redundant. You have the same code twice, with just different conditions. You can already see why this is bad: You typed the output strings differently without realising, so in some cases when the player is out of bounds the output will be GAME OVER, in some cases it will be Game OVER. This is a bug, and a very simple example of bugs being introduced or at least made much harder to find by redundant code.

To fix it you can either combine the logical expressions into one condition:

#Boundary check
if (player.xcor() > 300 or
    player.xcor() < -300 or
    player.ycor() > 300 or
    player.ycor() < -300):
    print("GAME OVER")
    quit()

Or, even better, define a function to check the bounds (this is way more readable and maintainable):

def is_player_in_bounds():
    return (player.xcor() < 300 and
            player.xcor() > -300 and
            player.ycor() < 300 and
            player.ycor() > -300)

Note that I inverted the logical expression, because I check for in bounds, not for out of bounds. Then you use it like this:

if not is_player_in_bounds():
    print("GAME OVER")
    quit()

#Collision checking
d= math.sqrt(math.pow(player.xcor()-goal.xcor(),2) + math.pow(player.ycor()-goal.ycor(),2))
if d < 20 :
    goal.setposition(random.randint(-300,300), random.randint(-300, 300))
    score = score + 1
    print ("\n" * 40)
    print("Your score is")
    print (score)

d is a very bad name for a variable. Since this looks like a calculation of Euler distance, I assume that it is supposed to mean distance, so better call your variable distance or at least dist.

You can write score += 1 here to make it shorter.

You might want to put the ouput code into a function (e. g. print_score()), to separate logic from I/O.

math.sqrt(math.pow(player.xcor()-goal.xcor(),2) + math.pow(player.ycor()-goal.ycor(),2)) is hard to read, because of the formatting, and because it is a long formula with variable names, namespaces and method/function calls. Better put it in a function:

def euler_distance(player, goal):
    distance_x = player.xcor() - goal.xcor()
    distance_y = player.ycor() - goal.ycor()
    return math.sqrt(distance_x ** 2 + distance_y ** 2)

Using a ** b for exponentiation is faster and looks cleaner than using math.pow(). Note that player and goal are local variables here, even if they have the same name as the global ones.

Use the function like this:

if euler_distance(player, goal) < 20:
    goal.setposition(random.randint(-300, 300), random.randint(-300, 300))
    score += 1
    print_score()

Even better if you would define the bounds somewhere as variables and access them that way.


Redundance

#Set speed
speed = 1

This comment tells you nothing about the code that the code does not already tell you. It just bloats the code and requires you to read more redundant information. When you read speed = 1, it is perfectly clear that the speed is being set. Comments should tell you why the code does something (if needed), while the code tells you what it does.


def turnleft():
    player.left(30)

def turnright():
    player.right(30)

These functions do not add any value. (Hint: Just after reviewing these functions I saw why they are defined, which is using them for event handling. In this case they do add value, but I will leave the info for understanding.) player.left(30) gives you more information than turnleft(). If you read turnleft() somewhere in your code, you do not know what is turned, you do not know how much it is turned. Whenever you want to write a function that only contains one or two lines, think about whether the one line or two lines would be more readable and give you more valuable information.

def increasespeed():
    global speed
    speed +=0.5

def decreasespeed():
    global speed
    speed -= 1

Here we have the same, but even worse: The functions change global state, which can make it much harder to find bugs. Try to avoid that whenever possible. Furthermore, speed += 0.5 and speed -= 1 already tell you that the speed is increased/decreased, and they tell you how much they are changed, which is valuable information. If your only reason for defining these functions is that you can easily change how much the speed is increased/decreased, define variables instead and do speed += SPEED_INCREASE. Writing it in uppercase later reminds you not to change it.


#Set keyboard binding
turtle.listen()
turtle.onkey(turnleft, "Left")
turtle.onkey(turnright, "Right")
turtle.onkey(increasespeed, "Up")
turtle.onkey(decreasespeed, "Down")

Since we said above that not defining these functions might be better, you can use lambdas as callbacks. Lambdas are anonymous functions, that means you do not have to define them with def, but you can just use them where you need them. In the cases where you just change a variable however, you will still need the function. This is what it could look like:

#Set keyboard binding
turtle.listen()
turtle.onkey(lambda: player.left(30), "Left")
turtle.onkey(lambda: player.right(30), "Right")
turtle.onkey(increasespeed, "Up")
turtle.onkey(decreasespeed, "Down")
\$\endgroup\$

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.