This is the first big project for me and I am not sure which is the correct way to structure the code yet. Is it okay to have so many classes and functions? What else should I improve?

import pygame
import sys
import random

clock = pygame.time.Clock()

font = pygame.font.SysFont('Monospace', 35)

BLACK = (0, 0, 0)
GREEN = (32, 238, 189)
YELLOW = (255, 230, 0)
RED = (250, 25, 78)
GREEN2 = (33, 239, 33)
WHITE = (255, 255, 255)

GRID_SIZE = [40, 30]
FPS = 20
screen = pygame.display.set_mode((WIDTH, HEIGHT))

score = 0

class BodyPart:

    def __init__(self, x, y, direction):
        self.dir_x, self.dir_y = direction
        self.x, self.y = x, y

    def set_direction(self, direction):
        self.dir_x = direction[0]
        self.dir_y = direction[1]

class Snake:

    def __init__(self):
        self.color = GREEN
        self.body = [BodyPart(TILE_SIZE * 3, int(HEIGHT / 2), (1, 0))]

    def append_body_part(self, count):
        for i in range(count):
            x, y, dir_x, dir_y = self.body[-1].x, self.body[-1].y, self.body[-1].dir_x, self.body[-1].dir_y
            x -= dir_x * TILE_SIZE
            y -= dir_y * TILE_SIZE
            self.body.append(BodyPart(x, y, (dir_x, dir_y)))

    def update_position(self):
        for body_part in self.body:
            body_part.x += body_part.dir_x * TILE_SIZE
            body_part.y += body_part.dir_y * TILE_SIZE
        z = len(self.body) - 1
        while z != 0:
            self.body[z].dir_x = self.body[z-1].dir_x
            self.body[z].dir_y = self.body[z-1].dir_y
            z -= 1

    def collision(self):
        head_x, head_y = self.body[0].x, self.body[0].y
        for body_part in snake.body[1:]:
            if body_part.x == head_x and body_part.y == head_y:
                    return True
        if head_x < 0 or head_x == WIDTH or head_y < 0 or head_y == HEIGHT:
            return True
        return False

class Food:

    def __init__(self):
        self.x = None
        self.y = None
        self.length_bonus = None
        self.color = None

    def display(self):
        pygame.draw.rect(screen, self.color, (self.x, self.y, TILE_SIZE, TILE_SIZE))

    def eaten(self):
        return (snake.body[0].x, snake.body[0].y) == (self.x, self.y)

    def __random_coords(self):
        matching = True
        while matching:
            self.x = random.randrange(TILE_SIZE, WIDTH - TILE_SIZE + 1, TILE_SIZE)
            self.y = random.randrange(TILE_SIZE, HEIGHT - TILE_SIZE + 1, TILE_SIZE)
            for body_part in snake.body:
                if self.x != body_part.x and self.y != body_part.y:
                    matching = False

    def __choose_random_food(self):
        self.foods = [self.__banana, self.__apple, self.__watermelon]

    def __banana(self):
        self.length_bonus = 1
        self.color = YELLOW

    def __apple(self):
        self.length_bonus = 2
        self.color = RED

    def __watermelon(self):
        self.length_bonus = 3
        self.color = GREEN2

def redraw_screen():
    global food_on_screen, food, score
    for body_part in snake.body:
        pygame.draw.rect(screen, snake.color, (body_part.x, body_part.y, TILE_SIZE, TILE_SIZE))
    if not food_on_screen:
        food = Food()
        food_on_screen = True
    if food.eaten():
        score += food.length_bonus
        food_on_screen = False
    text = font.render(f"Score: {score}", False, WHITE)
    screen.blit(text, (TILE_SIZE, TILE_SIZE))

snake = Snake()
food_on_screen = False
game_over = False

while not game_over:

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
        if event.type == pygame.KEYDOWN:
            if event.key == pygame.K_RIGHT and snake.body[0].dir_x != -1:
                snake.body[0].set_direction([1, 0])
            if event.key == pygame.K_LEFT and snake.body[0].dir_x != 1:
                snake.body[0].set_direction([-1, 0])
            if event.key == pygame.K_UP and snake.body[0].dir_y != 1:
                snake.body[0].set_direction([0, -1])
            if event.key == pygame.K_DOWN and snake.body[0].dir_y != -1:
                snake.body[0].set_direction([0, 1])


    if snake.collision():
        game_over = True


print(f"Your final score: {score}")


1 Answer 1


First of all, from an Object-Oriented perspective, you have objects that represent real objects within the game with behaviors and attributes, so on that note, well done! There are, however, other areas that need a bit of TLC.

You have an enumeration of colors sitting there as individually-defined constants. For a bit more type-safety, I would suggest persisting them in an Enum instead:

from enum import Enum

class Color(Enum):
    BLACK = (0, 0, 0)
    # other
    # colors
    # here

You can then access the enum constant for the color black as Color.BLACK and the tuple of RGB values as Color.BLACK.value.

Moving on to something a bit more functional, it doesn't make sense that each BodyPart has a direction. Conceptually speaking, the only thing in the game Snake that has a direction is the head, and each body part occupies the space of the body part in front of it per game tick.

I would suggest removing direction entirely from BodyPart, and storing the BodyPart objects within Snake as a linked list. Because in the end, you can still iterate through each BodyPart, but considering the 3 most important features:

  • The head, and its directional movement
  • The link between each BodyPart and the next, to determine movement of the BodyParts
  • The tail, for adding new BodyParts to the Snake

The linked list is the most meaningful data structure to use here.

Also, Food is a bit weird. Actually initializing a Food object is hidden behind private methods, which makes no logical sense. Food initialization should require parameters to define what type of food it is and where it is on the game. We can revisit the idea of enums and introduce a FoodType enum of tuples of form (length_bonus, Color) where we can once again use our Color enum from before, then require a FoodType parameter in init along with x and y. Then we can move the methods that generate a random Color and position out of that class and use those to generate the necessities for a Food object.

Lastly, I would highly recommend encapsulating x and y in all cases into a Point object. That would make it much easier to keep track of each piece's coordinates, and subsequently movements, collisions, etc.

class Point(object):
    def __init__(self, x, y):
        self.x = x
        self.y = y
    # __eq__ and other important methods here
  • \$\begingroup\$ Thanks, this was very helpful. But there is one thing that I don't understand. Why do I need to store my color constants in a class which inherits from Enum? To access a color I now need to say Color.BLACK.value instead of just BLACK which makes it more complicated. What is the purpouse of this? \$\endgroup\$
    – Pythenx
    Jul 2, 2019 at 9:59
  • \$\begingroup\$ Several reasons, which allude to the capabilities of the Enum type. The reason I gave first was type safety: you can check if parameters are Colors since Color is a declared class now. Enums are also iterable, flexible, hashable, and more efficiently interpreted. Follow the link in my answer to the documentation for Enum, you'll see more advantages. \$\endgroup\$
    – TCFP
    Jul 2, 2019 at 11:42

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.