Python OOP Whac-A-Mole program using turtle

Question

I am trying to create an object oriented program in Python for a whac-a-mole program.

I want to keep this simple where there are just a hammer and a mole and when the hammer turtle makes contact with the mole turtle.

I have created 4 classes, one for Game which is the super class, one for sprite which inherits from Turtle and one each for hammer and mole which both inherit from Sprite.

What I am asking is how to instantiate the mole and hammer turtles I have made use of inheritance but want to minimize the amount of methods I am using:

import random
import turtle
wn=turtle.Screen()
wn.addshape("moleSmall.gif")
wn.addshape("Mole.gif")
wn.addshape("Hammer.gif")
turtle.setup(800, 600)

    class Game():
        def __init__(self):
            self.score=0
            self.pointsDisplay=Points()

        def update_score(self,x,y):
            self.score = self.score + 2
            print("Score:", self.score)
            self.callDisplayScore()        
        def decrease_score(self,x,y):
            self.score = self.score -1
            print("Score: ", self.score)
            self.callDisplayScore()
        def callDisplayScore(self): 
            score = self.score
    class Sprite(turtle.Turtle):
        def __init__(self):
            turtle.Turtle.__init__(self)
            self.x=0
            self.y=0
            crash =False
            self.crash=crash
        def randomMove(self):        
            leftbound = -self.wn.window_width()/2
            rightbound = self.wn.window_width()/2
            topbound = self.wn.window_height()/2
            bottombound = -self.wn.window_height()/2
            rand_x = random.randint(leftbound , rightbound)
            rand_y = random.randint(bottombound, topbound) 
            self.mole.setpos(rand_x,rand_y)
        def collisionCheck(self,turtle1, turtle2):
            self.crash = False
            turtle1X = turtle1.xcor()
            turtle1Y = turtle1.ycor()
            turtle2X = turtle2.xcor()
            turtle2Y = turtle2.ycor()
            turtle1Pos = (int(turtle1X), int(turtle1Y))
            turtle2Pos = (int(turtle2X), int(turtle2Y))
            if turtle1Pos == turtle2Pos or turtle1X < turtle2X - 50 and \
                turtle1Y > turtle2Y - 50 and  turtle1X < turtle2X - 50 and\
                turtle1Y > turtle2Y + 50:
                self.crash = True
            return self.crash
    class Mole(Sprite):
        def __init__(self):
            Sprite.__init__(self)
            self.mole.pu()
            self.mole.shape("moleSmall.gif")
            self.x= self.mole.xcor()
            self.y = self.mole.ycor()
            self.randomMove()
    class Hammer(Sprite):
        def __init__(self):
            Sprite.__init__(self)
            self.counter = 0
            self.hammer.penup()
            self.hammer.shape("Hammer.gif")
            self.hammer.penup()
            leftbound = -self.wn.window_width()/2
            rightbound = self.wn.window_width()/2
            topbound = self.wn.window_height()/2
            bottombound = -self.wn.window_height()/2
            rand_x = random.randint(leftbound , rightbound)
            rand_y = random.randint(bottombound, topbound)
            self.hammer.setpos(rand_x,rand_y) 
        def HammerMove(self):
            self.hammer.pu()
            self.hammer.clear()
            self.wn.onscreenclick(self.hammer.goto)
            while self.collisionCheck(self.mole, self.hammer) == False:
                self.randomMove()
                if self.collisionCheck(self.mole, self.hammer) == True:
                    self.score = self.score + 1
                    print(self.score) 

Answer 1

Terminology

one [class] for Game which is the super class

What you probably mean is that it is the class that controls everything and such is on top of the logical class hierarchy. But when talking about super classes, you usually mean classes from which other classes inherit and thus are on top of (or further up) the inheritance hierarchy. In your code for example, Sprite is a super class of Mole.

White space

To make your code more readible, you should add blank lines appropriately, to separate logically related blocks of code. For example, add a blank line between methods, and two blank lines between classes. Inside of methods you can add single blank lines wherever it seems appropriate, for example around loops.

You should add spaces around operators (like self.pointsDisplay = Points() instead of self.pointsDisplay=Points()) and after commas (e. g. def update_score(self, x, y): instead of def update_score(self,x,y):), however not before commas, as you did here random.randint(leftbound , rightbound).

Refer to PEP 8, which is the Python style guide.

Naming

Your naming scheme is inconsistent. Python uses snake_case, not camelCase, for member names (see also PEP 8). You mixed both schemes, for both methods and member variables. It is always better to be consistent with one naming scheme, and prefered to follow the conventions of the language.

---

You are using x and y as parameter names. It is better to use speaking names that tell you what they are for or what they contain, e. g. position_x, position_y. In some cases x and y can be sufficient if the method name is explicit enough, like move_to_position(self, x, y), but I wouldn't say this is the case with update_score. Furthermore, you aren't even using the parameters in update_score and decrease_score.

---

update_score and decrease_score seems a bit confusing. Isn't decreasing also a kind of updating? Shouldn't the opposite be called increase_score? Maybe you can use just one method update_score for both cases, and control the amount how much it is increased or decreased with a parameter.

Logic

Your method callDisplayScore does not do anything, because all that happens when you call it is that a local variable is assigned a value, but it is not used afterwards. After the method had been called, the local variable does not exist anymore. So you could remove that method alltogether, and remove the calls to it from the methods update_score and decrease_score. Then all that remains in those two methods can basically be done without methods, because you can simply change the value of the score and then print it, which is hardly worth a method. Not using a method there will probably also make the code more readible, because it is hard to find a method name that describes well what these two lones of code do.

---

This is redundant:

crash =False
self.crash=crash

You can just write self.crash = False instead.

---

This expression is way too long to understand what it means:

if turtle1Pos == turtle2Pos or turtle1X < turtle2X - 50 and \
    turtle1Y > turtle2Y - 50 and  turtle1X < turtle2X - 50 and\
    turtle1Y > turtle2Y + 50:
    self.crash = True

Better save the result in a variable with a speaking name, or define a method that returns the result of the expression, again with a speaking name. Such a name could be is_colliding. Also using parantheses to group parts of the expression (even if they are technically not needed) can improve readability a lot.

---

This method also contains a lot of redundant logic, and also has not a very good name:

def collisionCheck(self,turtle1, turtle2):
    self.crash = False
    turtle1X = turtle1.xcor()
    turtle1Y = turtle1.ycor()
    turtle2X = turtle2.xcor()
    turtle2Y = turtle2.ycor()
    turtle1Pos = (int(turtle1X), int(turtle1Y))
    turtle2Pos = (int(turtle2X), int(turtle2Y))
    if turtle1Pos == turtle2Pos or turtle1X < turtle2X - 50 and \
        turtle1Y > turtle2Y - 50 and  turtle1X < turtle2X - 50 and\
        turtle1Y > turtle2Y + 50:
        self.crash = True
    return self.crash

Method names should be verbs (in most cases), so check_collision is better than collisionCheck. Even better: is_colliding or is_collision, because check_collision is not explicit about what a result of True means.

All of these variables you assign in the first 7 lines of the method are not needed. Your self.crash is not even used anywhere else, so a local variable (without self.) would be enough even if you needed the variable at all. In some cases it can be good to save values from method calls into variables in order to not have to call the methods multiple times, but if the method is just an accessor to a member variable, like turtle1.xcor(), the overhead from the method call is negligible, and readability is much more important as long as performance is not an issue. And instead of returning a boolean value depending in the result of a logical expression, just return the result of the expression instead.

Checking whether the position is the same (turtle1Pos == turtle2Pos) is obsolete if you then check with a threshold anyway. By the way, you have a bug here: turtle1X < turtle2X - 50 occures twice in the expression, perhaps on of the minuses should be a plus. And since turtle1Y > turtle2Y - 50 and turtle1Y > turtle2Y + 50 are combined with and I assume that you have another bug leading to false collisions in some cases. Here one of the > should be a <. You can simplify your collision detection logic by just checking whether the absolute distance is lower than your threshold. Optionally you could define the threshold somewhere as a variable or inject it as a parameter.

Simplifying the method, I get this:

def is_collision(self, turtle1, turtle2):
    return abs(turtle1.xcor() - turtle2.xcor()) < 50 and \
           abs(turtle1.ycor() - turtle2.ycor()) < 50