Recently I've been trying to learn more advanced things such as using classes and creating methods and such to implement OOP into my coding. This is the first time I've tried and I want opinion on what I could do to make this better or more efficient, or maybe what I'm doing wrong

Optional Read For context, I watched a beginner tutorial on YouTube on how to create a game, and then after I finished the tutorial, I decided to try and implement classes and objects. Essentially the game is just a spaceship (red rectangle) at the bottom of the screen that moves left and right to try and avoid the stars (white rectangles) that fall from the top of your screen to the bottom

Main Class:

import pygame
import random
import time

from Player import Player
from Star import Star


FONT = pygame.font.SysFont("comicsans", 30)

screen = pygame.display.set_mode((SCREEN_WIDTH, SCREEN_HEIGHT))
pygame.display.set_caption("Space Avoiders")

background = pygame.transform.scale(pygame.image.load("background1.jpg"), (SCREEN_WIDTH, SCREEN_HEIGHT + 15))
player = Player(SCREEN_WIDTH/2 - 20, SCREEN_HEIGHT - 60, 20, 60)

start_time = time.time()
elapsed_time = 0

star_spawn_rate = 2000
star_spawn = 1000
star_spawn_rate_decrease = 50
stars_to_spawn = 3
stars = []

hit = False

def starSpawner():
    for _ in range(3):
        randx = random.randint(0, 980)
        stars.append(Star(randx, -40, 20, 40))

def redrawGameWindow():
    screen.blit(background, (0, 0))
    time_text = FONT.render(f'Time: {round(elapsed_time)}s', 1, "white")
    screen.blit(time_text, (10, 10))

    for star in stars:


run = True
clock = pygame.time.Clock()
while run:
    star_spawn += clock.tick(60)
    elapsed_time = time.time() - start_time

    playerRect = pygame.Rect(player.x, player.y, player.width, player.height)

    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            run = False

    if star_spawn >= max(200, star_spawn_rate):
        star_spawn_rate -= star_spawn_rate_decrease
        star_spawn = 0

    for star in stars[:]:
        starRect = pygame.Rect(star.x, star.y, star.width, star.height)
        if star.y >= SCREEN_HEIGHT:
        elif star.y + star.height >= player.y and starRect.colliderect(playerRect):
            hit = True

    if hit:
        lose_text = FONT.render("You Lost!", 1, "white")
        screen.blit(lose_text, (SCREEN_WIDTH/2 - lose_text.get_width()/2, SCREEN_HEIGHT/2 - lose_text.get_height()/2))



Player Class:

import pygame

class Player:

def __init__(self, x, y, width, height):
    self.x = x
    self.y = y
    self.width = width
    self.height = height
    self.vel = 3

def draw(self, screen):
    pygame.draw.rect(screen, "red", (self.x, self.y, self.width, self.height))

def move(self):
    keys = pygame.key.get_pressed()
    if keys[pygame.K_LEFT]:
        self.x -= self.vel
    elif keys[pygame.K_RIGHT]:
        self.x += self.vel

Star Class:

import pygame

class Star:
    def __init__(self, x, y, width, height):
        self.x = x
        self.y = y
        self.width = width
        self.height = height
        self.vel = 5

    def draw(self, screen):
        pygame.draw.rect(screen, "white", (self.x, self.y, self.width, self.height))

    def fall(self):
        self.y += self.vel

1 Answer 1


Star and Player look lovely.

screen = ...

while run:

Ok, all that stuff needs to be packaged up within def main(): or something similar.


Imagine that you, or someone else, wanted to write a unit test for starSpawner. That module would need to import the current module. Which should happen without side effects, beyond the occasional function being defined.

Avoid such side effects. Every module should be safe to import from elsewhere.

Include a

if __name__ == `__main__`:

guard at the end, so the main() function is only invoked via $ python main.py, and not by import main.

Also, one could probably invent a more informative module name than "main".

def starSpawner():
def redrawGameWindow():

Pep-8 asks that you choose names like star_spawner and redraw_game_window.

    for star in stars[:]:

Iterating over stars.copy() so we can delete entries as we go is slightly tricky, and merits a # comment explaining it.

This code appears to achieve its design goals.

I would be willing to delegate or accept maintenance tasks on this codebase.

  • \$\begingroup\$ I appreciate this input very much! Like I said this is really the first time I've truly attempted migrating from a basic level to a more intermediate level of programming and although this program really isn't much, it's nice to hear that I didn't fail miserably trying aha! I've gone ahead and made all adjustments you mentioned, just one very last thing, if I were to create a lose screen for the game, one that has a play again button and a quit button, should that be done in the Main file, or should I put that in a separate class? \$\endgroup\$ Commented Aug 11, 2023 at 1:00
  • \$\begingroup\$ Create a lose.py module, or throw it all into the existing module; I don't see that it makes a giant difference. One big module seems simpler ATM. Here's a rule that can help you with such decisions: Write a module-level """docstring""" that starts with a single sentence describing the module's single responsibility. Then ask yourself if def lose(): fits within what that sentence describes, or if a new module is warranted. Same approach works with class docstrings and with method docstrings, when deciding to split something. \$\endgroup\$
    – J_H
    Commented Aug 11, 2023 at 5:14

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.