Python Asteroids on GitHub

I want to while away a little time. What can we find that’s right-sized?

How about caching the ships that we display as available? Or would it be better to cache just one and provide a “goto” in the MovableLocation?

Here’s the code that displays available ships:

    def draw_available_ships(self):
        for i in range(0, ShipFleet.ships_remaining):
            self.draw_available_ship(i)

    def draw_available_ship(self,i):
        position = i*Vector2(35, 0)
        ship = Ship(Vector2(55,100) + position)
        ship.angle = 90
        ship.draw(self.screen)

I think we should cache just one and move it.

class Game:
    available_ship = Ship(Vector2(0, 0))

    def draw_available_ships(self):
        for i in range(0, ShipFleet.ships_remaining):
            self.draw_available_ship(self.available_ship, i)

    def draw_available_ship(self, ship, i):
        position = i*Vector2(35, 0)
        ship.move_to(Vector2(55, 100) + position)
        ship.draw(self.screen)

class Ship:
    def move_to(self, vector):
        self.location.move_to(vector)

class MovableLocation:
    def move_to(self, vector):
        self.position = vector

That does the job nicely. Commit: use cached copy of Ship to display available ships.

A fine change, I guess. We have no evidence that creating all those ships was actually a problem, but it seems better this way.

Now maybe I should look at users of the setter property and change them.

I find lots of tests setting position. Let’s change those to move_to calls.

I change about 7000 saucer.position = to move_to. Green. Commit: change position assignments on Saucer to use move_to, remove position setter.

I’ll look for other classes that have the position setter.

I add move_to to Asteroid and remove the setter. Commit: remove asteroid position setter.

Now what about the Velocity setters?

I implement:

class Saucer:
    def accelerate_to(self, velocity):
        self.location.accelerate_to(velocity)

class MovableLocation:
    def accelerate_to(self, velocity):
        self.velocity = velocity

Replace all the calls to the velocity setter, and: Commit: Remove saucer velocity setter, replace with accelerate_to(velocity).

Summary

I think this is better. The workings of MovableLocation are a bit better hidden by move_to and accelerate_to than by allowing people to set its member variables. Who knows, we might come up with a better way of moving.

We still are accessing its position and velocity and doing it directly, but if we ever care about that we can cover them with properties. If I didn’t trust myself, I’d cover them now to avoid setting from outside.

I do think this is better, in small ways. Afternoons are for small ways.

See you next time!