Python 192 - Making Duplication
What if we made all three
draw
commands look alike?
I’ve changed my three draw methods to look alike:
class Asteroid(Flyer):
def draw(self, screen):
points = raw_rocks[self._rock_index]
scale = self.radius / 4
position = self.position
angle = 0
adjusted = [point.rotate(-angle) * scale + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
class Saucer(Flyer):
def draw(self, screen):
points = raw_saucer_points
scale = self._scale
position = self.position
angle = 0
adjusted = [point.rotate(-angle) * scale + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
class Ship(Flyer):
def draw(self, screen):
points = self.select_ship_source()
scale = 4 * u.SCALE_FACTOR * self._drop_in
position = self.position
angle = self._angle
adjusted = [(point.rotate(-angle) * scale) + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
Note that although two of them don’t really rotate, I included the angle so as to make them look more alike.
Now we can extract a class. I think this is properly called Method Object:
class Ship(Flyer):
def draw(self, screen):
points = self.select_ship_source()
scale = 4 * u.SCALE_FACTOR * self._drop_in
position = self.position
angle = self._angle
LinePainter(points, scale, position, angle).draw(screen)
class LinePainter:
def __init__(self, points, scale, position, angle = 0):
self._points = points
self._scale = scale
self._position = position
self._angle = angle
def draw(self, screen):
adjusted = [(point.rotate(-self._angle) * self._scale) + self._position for point in self._points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
And use it in Asteroid and Saucer:
class Asteroid(Flyer):
def draw(self, screen):
points = raw_rocks[self._rock_index]
scale = self.radius / 4
position = self.position
LinePainter(points, scale, position).draw(screen)
class Saucer(Flyer):
def draw(self, screen):
points = raw_saucer_points
scale = self._scale
position = self.position
LinePainter(points, scale, position).draw(screen)
Is this better? In favor of the idea, it shows clearly that all the cases are the same, and it isolates the complicated code, the adjustment. Opposed, it creates and destroys three LinePainter instances every 60th of a second. I think there’s a rule somewhere that says “Trust your garbage collector”, but it is a valid concern: it’s certainly not as efficient as it was.
A related issue is that it unconditionally does the rotate, even though two out of three calls will have zero as the rotation.
We could fix that internally now, if we care to:
def draw(self, screen):
if self._angle:
adjusted = [(point.rotate(-self._angle) * self._scale) + self._position for point in self._points]
else:
adjusted = [(point * self._scale) + self._position for point in self._points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
That’s certainly faster, but the big cost is surely in creating all those LinePainters and burning them. Alternatively, we could cache a LinePainter and then pass in all the parameters to the draw method.
Or we could just make a free-standing function somewhere that did this work.
How about that? We’ll extract one:
class Ship(Flyer):
def draw(self, screen):
points = self.select_ship_source()
scale = 4 * u.SCALE_FACTOR * self._drop_in
position = self.position
angle = self._angle
self.draw_lines(screen, points, position, scale, angle)
def draw_lines(self, screen, points, position, scale, angle):
adjusted = [(point.rotate(-angle) * scale) + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
Promote the function to first class and put it in with the raw points.
class Ship(FLyer):
def draw(self, screen):
points = self.select_ship_source()
scale = 4 * u.SCALE_FACTOR * self._drop_in
position = self.position
angle = self._angle
draw_lines(screen, points, position, scale, angle)
raw_object_points.py
def draw_lines(screen, points, position, scale, angle = 0):
adjusted = [(point.rotate(-angle) * scale) + position for point in points]
pygame.draw.lines(screen, "white", False, adjusted, 3)
And use it:
class Saucer(Flyer):
def draw(self, screen):
points = raw_saucer_points
scale = self._scale
position = self.position
draw_lines(screen, points, position, scale)
class Asteroid(Flyer):
def draw(self, screen):
points = raw_rocks[self._rock_index]
scale = self.radius / 4
position = self.position
draw_lines(screen, points, position, scale)
So what do we think about this? Is it worth creating a top-level function to cover the points adjustment and subsequent drawing?
I don’t know. I like it better than the Method Object, because it doesn’t create and destroy little objects all over, and, given that you can tolerate a top-level function, it’s a bit simpler. It covers the complicated adjustment code, which is doing all the translation, rotation, and scaling that we need. It might also bring out issues that are more clear because of the simpler code. For example, the scale
of each of these is calculated differently, and somewhat magically.
Of course, with just three objects in hand, all this is more than we need. But if the Flyer objects were going to proliferate, centralizing this code would be a help. And observing the oddness, such as the different ways of getting the scale, would be of some value.
And, of course, there is the hidden advantage: we could make the asteroids spin if we wanted to.
I think I’ll allow this. Commit: asteroid, saucer, and ship centralize drawing in draw_lines function.
Summary
Just a little finger exercise, looking at different ways of dealing with some methods that are quite similar and yet different. We first made them look more alike, a trick that I picked up from the great Kent Beck. Then we extracted the common bits, first into a class, which was OK but seemed like too much, and then just into a top-level function. I think that’s sufficiently Pythonic to be allowed.
Better? YMMV, but I think it is, because it has centralized the complex rotation scaling translation code into a single place. Fewer chances for error.
See you next time!