Python 187 - In My Defense ...!
At the time I worked out how to draw the game objects, I was new to Pygame and all its works and all its pomps. We deserve better. Refactoring FTW!!
- Nota Bene
- This article is long, because it includes many copies of the same code, as I evolve it incrementally from its current somewhat messy form to a much nicer much less messy form.
-
I would advise reading the words, then in cases that interest you, scan the code to see the simple change that was made.
-
Why? Because in a series of small, simple, safe steps, we’re going to refactor some messy code into some very nice code. It’s easy, smooth, and the techniques are available to us all.
-
Check it out! It goes very well!
- Good intentions …
- When I was just starting with Python, Pygame, and the asteroids program, I had good intentions but not very much experience with the language and library. And, I may have moved too fast. I prefer to blame ignorance, however, than any bad habits that I may or may not have.
Pygame seems to prefer a form of graphics where you “blit” a surface to the screen. That is, you have in memory a rectangle of pixels, and when it is time to draw whatever they represent, you tell Pygame to blit them to the location where you want the thing drawn. Blit operations are generally very fast, or at least they sued to be decades ago, so I suppose they are now, as well.
So it can make sense. The saucer, for example, is about a dozen lines, and an asteroid is 10, 11, or 12, depending on the particular design. So if you can draw those dozen lines onto an array of pixels somewhere, and blast them onto the screen in one operation, it can be “better”. Of course, if the object were more complicated, such as a nicely colored complicated sprite, it could be even more advantageous. Something like this:
That’s an early picture from a Codea version of asteroids that I did back in 2020. Anyway, just about the only way to draw that ship would probably be a blit operation.
I say probably. Pygame does have the ability to draw to any Surface, and the screen is a surface. So in principle, we could draw anything, even that fancy ship, directly to the screen. It just seemed to me when I was learning Pygame that the done thing was to draw the objects once to internal small surfaces, and then blit them as needed. Here’s the code for drawing an asteroid, for example:
def draw(self, screen):
top_left_corner = self.position - self._offset
screen.blit(self._surface, top_left_corner)
The _surface member has the asteroid all nicely drawn on it already, once and for all.
However.
- In the distant past …
-
Yesterday we were tasked with changing the scale of the game to provide a little more room between objects, making the game more playable. After a few quick attempts, I found a decent way to do that, involving a new universal constant
SCALE_FACTOR. Here’s how we used it in Asteroid, probably the most complicated case, since there are three different sizes of asteroid:
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._offset = Vector2(self.radius, self.radius)
self._surface = SurfaceMaker.asteroid_surface(u.SCALE_FACTOR * self.radius * 2)
You’ll note two references to u.SCALE_FACTOR there, one when computing the radius of the asteroid (based on its size, 0, 1, or 2), and again in the call to SurfaceMaker.asteroid_surface.
Reviewing asteroid_surface:
class SurfaceMaker:
@staticmethod
def asteroid_surface(actual_size):
raw_rock_points = SurfaceMaker.get_next_shape()
raw_points_span = 8
raw_points_offset = Vector2(4, 4)
scale = actual_size / raw_points_span
room_for_fat_line = 2
surface_size = actual_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface((surface_size, surface_size),
raw_points_offset, scale, raw_rock_points)
return surface
We see that we’re calling the parameter actual_size, and we undertake some weird calculations to get the size of the surface that we want.
- This is not really very good …
- Not horrid but not great.
Issues include:
- The line definitions we have for asteroids use coordinates from -4 to +4 in both x and y. That’s because we found the original definitions from the 1980s game, and that’s what the coordinates were.
- The Asteroid code thinks that it wants the
actual_sizeto be 16, 32, or 64 pixels (before our new scaling). - Because we draw a line with width 3 pixels, so that it’ll be visible on screen, we need a little extra room in our surface.
Therefore …
- Since the span of points in the raw definition is 8 (-4 through +4), if we want size 16, the asteroid should be drawn at scale
actual_size / raw_points_span, or 16 / 8, or 2. - Meanwhile, the surface size should be (16+2, 16+2) to allow for the fat line.
All that feeds into the create_scaled_surface method, which draws lines into a surface of the desired size:
class SurfaceMaker:
@staticmethod
def create_scaled_surface(dimensions, offset, scale_factor, *point_lists):
surface = pygame.Surface(dimensions)
surface.set_colorkey((0, 0, 0))
for point_list in point_lists:
SurfaceMaker.draw_adjusted_lines(offset, point_list, scale_factor, surface)
return surface
@staticmethod
def draw_adjusted_lines(offset, point_list, scale_factor, surface):
adjusted = [SurfaceMaker.adjust(point, offset, scale_factor) for point in point_list]
pygame.draw.lines(surface, "white", False, adjusted, 3)
@staticmethod
def adjust(point, center_adjustment, scale_factor):
return (point + center_adjustment) * scale_factor
The adjust function manually translates the raw corner points to translated and scaled points, adjusting them to accommodate pygame’s top-left orientation and expanding them to the desired scale.
By the time we get to the drawing here, all the objects, asteroid, saucer, and ship, are much the same. Unfortunately, each one is a bit different. There are four asteroid shapes and they are sized -4 to +4. The saucer has only one shape, which spans -5 to +5 in x, and -3 to +3 in y. The ship shape has two separate parts, and together they span -7 to +7 in x, -4 to +4 in y.
These differences are accommodated by having separate surface creation setup methods for each. Here are all three for us to look at together.
class SurfaceMaker:
@staticmethod
def asteroid_surface(actual_size):
raw_rock_points = SurfaceMaker.get_next_shape()
raw_points_span = 8
raw_points_offset = Vector2(4, 4)
scale = actual_size / raw_points_span
room_for_fat_line = 2
surface_size = actual_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface((surface_size, surface_size),
raw_points_offset, scale, raw_rock_points)
return surface
@staticmethod
def saucer_surface(saucer_size):
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = saucer_size.x / raw_points_span.x
room_for_fat_line = Vector2(0, 2)
saucer_surface = SurfaceMaker.create_scaled_surface(
saucer_size + room_for_fat_line, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
@staticmethod
def ship_surfaces(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
ship_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points)
accelerating_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return ship_surface, accelerating_surface
This is all “reasonable” but it’s not ideal, and would be a terror if we had more than just three kinds of objects to draw. If each one required a custom method like these … that would be bad. These methods look like someone copied the first one, pasted it to the second, and then bashed it until it worked.
Don’t look at me like that. I know you’ve done it too. But, no, that’s not really a recommended practice, is it?
It seems to me that two possible improvements are desirable here.
First, we would like to have this code better factored so as to remove all the duplication, preserving whatever uniqueness really needs preserving. We would hope that that would be none.
Second, we would probably benefit from some kind of common data structure, including hit radius, scaling information, the lines, and whatever else we need to turn creation of these shapes from a procedural problem into a data problem.
- Can you clarify that?
-
It seems to me that when we have three things, asteroid, saucer, ship, and three procedures that are quite similar, that the problem can likely be refactored into one common procedure with all the variances recorded as calling parameters, and most likely, in small tables.
Let’s try to get there.
We’ll proceed by trying to make these methods more similar.
The ship method creates two surfaces, the others only one. Let’s make two ship surface makers, so that the ship will be like the others (except that it calls two methods to get its two surfaces).
That’s easy enough here:
@staticmethod
def ship_surface(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
ship_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points)
return ship_surface
@staticmethod
def acceleration_surface(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
acceleration_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return acceleration_surface
- Oops
- I could have and should have removed
raw_ship_pointsfrom the call above. It is harmless. It disappears somewhere later on.
Now in Ship:
class Ship(Flyer):
def __init__ ...
...
self._ship_surface = SurfaceMaker.ship_surface(ship_size)
self._ship_accelerating_surface =
SurfaceMaker.acceleration_surface(ship_size)
I see that the names don’t match. Fix the method to accelerating to accommodate Ship’s preference.
class Ship(Flyer):
...
self._ship_accelerating_surface = SurfaceMaker.accelerating_surface(ship_size)
class SurfaceMaker:
@staticmethod
def accelerating_surface(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
accelerating_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return accelerating_surface
Works, of course. Commit: Refactor for two separate SurfaceMaker calls to create ship’s two surfaces.
Now let’s make those four methods more similar. We’re starting here now:
@staticmethod
def accelerating_surface(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
accelerating_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return accelerating_surface
@staticmethod
def asteroid_surface(actual_size):
raw_rock_points = SurfaceMaker.get_next_shape()
raw_points_span = 8
raw_points_offset = Vector2(4, 4)
scale = actual_size / raw_points_span
room_for_fat_line = 2
surface_size = actual_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface((surface_size, surface_size),
raw_points_offset, scale, raw_rock_points)
return surface
@staticmethod
def saucer_surface(saucer_size):
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = saucer_size.x / raw_points_span.x
room_for_fat_line = Vector2(0, 2)
saucer_surface = SurfaceMaker.create_scaled_surface(
saucer_size + room_for_fat_line, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
@staticmethod
def ship_surface(ship_size):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = ship_size.x / raw_points_span.x
ship_surface = SurfaceMaker.create_scaled_surface(
ship_size, raw_points_offset, scale_factor, raw_ship_points)
return ship_surface
Let’s make them all use raw_points_span as a vector, specified first. I’ll start with asteroid. Here’s my first step, with saucer_surface for comparison:
@staticmethod
def asteroid_surface(actual_size):
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = actual_size / raw_points_span.x
raw_rock_points = SurfaceMaker.get_next_shape()
room_for_fat_line = 2
surface_size = actual_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface((surface_size, surface_size),
raw_points_offset, scale_factor, raw_rock_points)
return surface
@staticmethod
def saucer_surface(saucer_size):
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = saucer_size.x / raw_points_span.x
room_for_fat_line = Vector2(0, 2)
saucer_surface = SurfaceMaker.create_scaled_surface(
saucer_size + room_for_fat_line, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
asteroid_surface, unlike all the others, is passed a scalar actual_size, not a vector. Let’s change that.
- Why?
-
Our mission is to make these methods all look the same. When we’re done, we’ll be able to combine them in some useful way. To do that, we change each one to be more and more similar to the others.
Asteroid __init__ needs to change:
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._offset = Vector2(self.radius, self.radius)
self._surface =
SurfaceMaker.asteroid_surface(u.SCALE_FACTOR * self.radius * 2)
We want to pass a vector on that last line. Extract Variable:
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._offset = Vector2(self.radius, self.radius)
scaled_size = u.SCALE_FACTOR * self.radius * 2
self._surface = SurfaceMaker.asteroid_surface(scaled_size)
Make the vector, pass it in:
class Asteroid(Flyer):
def __init__(self, size=2, position=None):
self.size = max(0, min(size, 2))
self._score = u.ASTEROID_SCORE_LIST[self.size]
self.radius = [16, 32, 64][self.size] * u.SCALE_FACTOR
position = position if position is not None else Vector2(0, random.randrange(0, u.SCREEN_SIZE))
angle_of_travel = random.randint(0, 360)
velocity = u.ASTEROID_SPEED.rotate(angle_of_travel)
self._location = MovableLocation(position, velocity)
self._offset = Vector2(self.radius, self.radius)
scaled_size = u.SCALE_FACTOR * self.radius * 2
asteroid_size = Vector2(scaled_size, scaled_size)
self._surface = SurfaceMaker.asteroid_surface(asteroid_size)
Change SurfaceMaker to expect the vector:
@staticmethod
def asteroid_surface(actual_size: Vector2):
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = actual_size.x / raw_points_span.x
raw_rock_points = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
surface_size = actual_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(surface_size, raw_points_offset, scale_factor, raw_rock_points)
return surface
Green. Commit: refactoring asteroid_surface for similarity to others.
Let’s rename all the parameters to be object_size, just to make the code look more similar.
@staticmethod
def accelerating_surface(object_size: Vector2):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
accelerating_surface = SurfaceMaker.create_scaled_surface(
object_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return accelerating_surface
@staticmethod
def asteroid_surface(object_size: Vector2):
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
raw_rock_points = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
surface_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(surface_size, raw_points_offset, scale_factor, raw_rock_points)
return surface
@staticmethod
def saucer_surface(object_size: Vector2):
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
room_for_fat_line = Vector2(0, 2)
saucer_surface = SurfaceMaker.create_scaled_surface(
object_size + room_for_fat_line, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
@staticmethod
def ship_surface(object_size: Vector2):
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
ship_surface = SurfaceMaker.create_scaled_surface(
object_size, raw_points_offset, scale_factor, raw_ship_points)
return ship_surface
Commit: renames for similarity.
Some of these guys have room for fat line and some do not. Let’s first extract:
@staticmethod
def asteroid_surface(object_size: Vector2):
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
raw_rock_points = SurfaceMaker.get_next_shape()
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_rock_points)
return surface
@staticmethod
def saucer_surface(object_size: Vector2):
room_for_fat_line = Vector2(0, 2)
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
saucer_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
I also moved room_for_fat_line to the top. Commit: changes for similarity.
Make the other two methods look the same by adding in room_for_fat_line as a zero vector, of course. Now the four methods are even more similar!
@staticmethod
def accelerating_surface(object_size: Vector2):
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
accelerating_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_ship_points, raw_flare_points)
return accelerating_surface
@staticmethod
def asteroid_surface(object_size: Vector2):
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
raw_rock_points = SurfaceMaker.get_next_shape()
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_rock_points)
return surface
@staticmethod
def saucer_surface(object_size: Vector2):
room_for_fat_line = Vector2(0, 2)
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
saucer_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_saucer_points)
return saucer_surface
@staticmethod
def ship_surface(object_size: Vector2):
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
ship_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_ship_points)
return ship_surface
These are really quite similar. There’s that selection in the asteroid case. Move it up to the top:
@staticmethod
def asteroid_surface(object_size: Vector2):
raw_rock_points = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_rock_points)
return surface
Ha! Notice that the other guys all name their points down in the bottom call. Let’s first rename the asteroid case thus, a new temp points_to_draw.
@staticmethod
def asteroid_surface(object_size: Vector2):
points_to_draw = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
In the others, extract the points from the create_scaled_surface call to a variable of the same name, and move it to the top:
No, wait. I just noticed that I’m still passing both points sets for one ship case. I mentioned that above. Change it:
@staticmethod
def accelerating_surface(object_size: Vector2):
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
accelerating_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, raw_flare_points)
return accelerating_surface
Commit: remove unneeded parameter.
Now the extract / move of points_to_draw:
@staticmethod
def accelerating_surface(object_size: Vector2):
points_to_draw = raw_flare_points
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
accelerating_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return accelerating_surface
Commit: extract points_to_draw.
Similarly:
@staticmethod
def saucer_surface(object_size: Vector2):
points_to_draw = raw_saucer_points
room_for_fat_line = Vector2(0, 2)
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
saucer_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return saucer_surface
Commit: extract points_to_draw.
Again:
@staticmethod
def ship_surface(object_size: Vector2):
points_to_draw = raw_ship_points
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
ship_surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return ship_surface
Commit: extract points_to_draw.
Rename the various return variables, whatever_surface to surface, to get the return names alike, and I think now our four methods are all the same:
def accelerating_surface(object_size: Vector2):
points_to_draw = raw_flare_points
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
@staticmethod
def asteroid_surface(object_size: Vector2):
points_to_draw = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
@staticmethod
def saucer_surface(object_size: Vector2):
points_to_draw = raw_saucer_points
room_for_fat_line = Vector2(0, 2)
raw_points_span = Vector2(10, 6)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
@staticmethod
def ship_surface(object_size: Vector2):
points_to_draw = raw_ship_points
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
Now we can extract the duplication, starting from the raw_points_offset down. Extract Method and PyCharm does the rest:
@staticmethod
def accelerating_surface(object_size: Vector2):
points_to_draw = raw_flare_points
room_for_fat_line = Vector2(0, 0)
raw_points_span = Vector2(14, 8)
return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)
@staticmethod
def asteroid_surface(object_size: Vector2):
points_to_draw = SurfaceMaker.get_next_shape()
room_for_fat_line = Vector2(2, 2)
raw_points_span = Vector2(8, 8)
return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)
@staticmethod
def saucer_surface(object_size: Vector2):
points_to_draw = raw_saucer_points
room_for_fat_line = Vector2(0, 2)
raw_points_span = Vector2(10, 6)
return SurfaceMaker.create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line)
@staticmethod
def create_desired_surface(points_to_draw, object_size, raw_points_span, room_for_fat_line):
raw_points_offset = raw_points_span / 2
scale_factor = object_size.x / raw_points_span.x
expanded_size = object_size + room_for_fat_line
surface = SurfaceMaker.create_scaled_surface(
expanded_size, raw_points_offset, scale_factor, points_to_draw)
return surface
Commit: extract duplicate code to method.
We’re not fully done, but we’re done for now. There’s a lot of code here, though not much change. More than enough for you to read.
Summary
Aside from entering simple statements like
room_for_fat_line = Vector2(0, 0)
almost all the changes here were PyCharm machine refactorings, pretty much guaranteed to work. And there are some tests for this stuff as well. The difference between where we started and where we are now is profound. We started with three methods that were oddly similar but quite different. We wind up with four tiny methods that just set up three values and then call the same method with those three values.
All that complexity has now boiled down to three data items, a list of points, a size vector, and a fat lines vector, passed to a single five line method.
I think next time we’ll see about representing those three input parameters more reasonably, with data rather than procedure. But we’re good for now.
A very noticeable improvement. And all done in simple mechanical steps. Nice!
See you next time!