"""
 Sample Python/Pygame Programs
 Simpson College Computer Science
 http://programarcadegames.com/
 http://simpson.edu/computer-science/

 Explanation video: http:

"""

# Import a library of functions called 'pygame'
import pygame

## draw_snowman is what I want to put in a class
def draw_snowman(screen, x, y):
    """ --- Function for a snowman ---
    Define a function that will draw a snowman at a certain location.
    """
    pygame.draw.ellipse(screen, WHITE, [35 + x, 0 + y, 25, 25])
    pygame.draw.ellipse(screen, WHITE, [23 + x, 20 + y, 50, 50])
    pygame.draw.ellipse(screen, WHITE, [0 + x, 65 + y, 100, 100])

# Initialize the game engine
pygame.init()

# Define the colors we will use in RGB format
BLACK = [0, 0, 0]
WHITE = [255, 255, 255]

# Set the height and width of the screen
size = [400, 500]
screen = pygame.display.set_mode(size)

# Loop until the user clicks the close button.
done = False
clock = pygame.time.Clock()

def main():
    while not done:
    
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                done = True
    
        # Clear the screen and set the screen background
        screen.fill(BLACK)
    
        # Snowman in upper left
        draw_snowman(screen, 10, 10)
    
        # Snowman in upper right
        draw_snowman(screen, 300, 10)
    
        # Snowman in lower left
        draw_snowman(screen, 10, 300)
    
        # Go ahead and update the screen with what we've drawn.
        # This MUST happen after all the other drawing commands.
        pygame.display.flip()
    
        # This limits the while loop to a max of 60 times per second.
        # Leave this out and we will use all CPU we can.
        clock.tick(60)
    
    
    # Be IDLE friendly
    pygame.quit()
    
if __name__ == "__main__":
    main()

Now if I make a class such as -

import pygame

# Define the colors we will use in RGB format
BLACK = [0, 0, 0]
WHITE = [255, 255, 255]

class Snowman():
    def __init__(self):
        # --- Class Attributes ---
        # Snowman position
        self.x = 0
        self.y = 0
        
        # Snowman's vector
        self.change_x = 0
        self.change_y = 0
        
        # Snowman size
        self.size = 10
        
        # Snowman color
        self.color = [255,255,255]
        
    # --- Class Methods ---
    def move(self):
        self.x += self.change_x
        self.y += self.change_y
    
    # --> Should I pass pygame/pygame.draw/pygame.draw.ellipse
    #     to the draw method? Or is this proper as is?
    def draw(self, screen):
        pygame.draw.ellipse(screen, WHITE, [35 + self.x, 0 + self.y, 25, 25])
        pygame.draw.ellipse(screen, WHITE, [23 + self.x, 20 + self.y, 50, 50])
        pygame.draw.ellipse(screen, WHITE, [0 + self.x, 65 + self.y, 100, 100])

And change the original code to this -

"""
 Sample Python/Pygame Programs
 Simpson College Computer Science
 http://programarcadegames.com/
 http://simpson.edu/computer-science/

 Explanation video: http:

"""

# Import a library of functions called 'pygame'
import pygame
from P10_Graphics import Graphics

def main():
    snowman1 = Graphics.Snowman()
    snowman2 = Graphics.Snowman()
    snowman3 = Graphics.Snowman()
    
    # Initialize the game engine
    pygame.init()
    
    # Define the colors we will use in RGB format
    BLACK = [0, 0, 0]
    WHITE = [255, 255, 255]
    
    # Set the height and width of the screen
    size = [400, 500]
    screen = pygame.display.set_mode(size)
    
    # Loop until the user clicks the close button.
    done = False
    clock = pygame.time.Clock()    
    
    while not done:
    
        for event in pygame.event.get():
            if event.type == pygame.QUIT:
                done = True
    
        # Clear the screen and set the screen background
        screen.fill(BLACK)
    
        # Snowman in upper left
        snowman1.x = 10
        snowman1.y = 10
        snowman1.draw(screen)
    
        # Snowman in upper right
        snowman2.x = 300
        snowman2.y = 10
        snowman2.draw(screen)
    
        # Snowman in lower left
        snowman3.x = 10
        snowman3.y = 300
        snowman3.draw(screen)
    
        # Go ahead and update the screen with what we've drawn.
        # This MUST happen after all the other drawing commands.
        pygame.display.flip()
    
        # This limits the while loop to a max of 60 times per second.
        # Leave this out and we will use all CPU we can.
        clock.tick(60)
    
    
    # Be IDLE friendly
    pygame.quit()
    
if __name__ == "__main__":
    main()

Should the draw() take other parameters? Is this right? Is there a better way to write this particular piece of code?

In an ideal world maybe Snowman shouldn't need to know about pygame, but that starts taking you down a rabbit hole of separating the logical data of a snowman from the visual appearance of a snowman, possibly not useful in this situation, especially given the reliance on the ellipse() function. If you liked you could have some sort of separate PyGameSnowman object that handles the rendering, reading data from the Snowman and rendering it accordingly.

In general code review terms:

• you shouldn't set the x/y values to constant values each time through the loop
• change_x / change_y are perhaps better described as 'velocity'
• you don't call `move()` so this isn't used anyway
• you might consider having a data structure that stores all your snowmen so that it extends to any number of snowmen in future

In an ideal world maybe Snowman shouldn't need to know about pygame, but that starts taking you down a rabbit hole of separating the logical data of a snowman from the visual appearance of a snowman,

I'd like to go down that rabbit hole.

especially given the reliance on the ellipse() function. If you liked you could have some sort of separate PyGameSnowman object that handles the rendering, reading data from the Snowman and rendering it accordingly.

Some example code of that, if you don't mind.

In general code review terms:

• you shouldn't set the x/y values to constant values each time through the loop
• change_x / change_y are perhaps better described as 'velocity'
• you don't call `move()` so this isn't used anyway
• you might consider having a data structure that stores all your snowmen so that it extends to any number of snowmen in future

• Agree.
• Agree.
• It'll be used later, in a different tutorial.
• An array?

class Snowman(object):
    """
    as above, but no draw() method - this is purely for logic, and
    is no longer anything to do with Pygame or graphics"""
    pass  # the rest of your code here

class IPygameRenderable(object):
    """Interface for objects renderable in Pygame"""
    def draw(self, pygame_screen):
        """Must be implemented by derived class"""
        raise NotImplementedError

class PygameSnowman(IPygameRenderable):
    def __init__(self, snowman):
        # Link to the object that tells us where to render,
        # and any other important traits that we may need to show
        self.snowman = snowman

    def draw(self, pygame_screen):
        pygame.draw.ellipse(pygame_screen, WHITE, [35 + self.snowman.x, 0 + self.snowman.y, 25, 25])
        pygame.draw.ellipse(pygame_screen, WHITE, [23 + self.snowman.x, 20 + self.snowman.y, 50, 50])
        pygame.draw.ellipse(pygame_screen, WHITE, [0 + self.snowman.x, 65 + self.snowman.y, 100, 100])

def main():
    all_snowmen = []
    all_snowmen.append(Graphics.Snowman())
    all_snowmen.append(Graphics.Snowman())
    all_snowmen.append(Graphics.Snowman())

    all_drawable = []
    for snowman in all_snowmen:
        drawable = PygameSnowman(snowman)
        all_renderable.append(drawable)

    # ...etc ...
    while not done:
        # ...etc...
        for snowman in all_snowmen:
            snowman.move()  # or .update(), or whatever other logic you have

        for drawable in all_drawable:
            drawable.draw(screen)