Let’s see if we can reduce accesses to the specialized lists in Fleets. As so often happens, the next step isn’t in the direction I expect. That’s OK.
The Fleets object now holds some specialized Fleets, and some vanilla ones. It initializes like this:
class Fleets: def __init__(self, asteroids=None, missiles=None, saucers=None, saucer_missiles=None, ships=None): asteroids = asteroids if asteroids is not None else  missiles = missiles if missiles is not None else  saucers = saucers if saucers is not None else  saucer_missiles = saucer_missiles if saucer_missiles is not None else  ships = ships if ships is not None else  self.fleets = ( Fleet(asteroids), Fleet(missiles), SaucerFleet(saucers), MissileFleet(saucer_missiles, u.SAUCER_MISSILE_LIMIT), ShipFleet(ships), Fleet(), # explosions not used Fleet()) self.thumper = Thumper(self.beat1, self.beat2) self.score = 0
Oh, we don’t need that score any more, do we?
I’m already distracted from what I thought I’d be doing, but this is worth going after. I’ve checked, and both Saucer and Asteroid add score by adding a Score object via Fleets, to be eaten by the ScoreKeeper. However Fleets is also maintaining the score:
class Fleets: def add_score(self, score): self.others.append(score) self.score += score.score
Let’s just start removing. I think a lot of tests are going to break, and I think we can fix it readily, if inefficiently.
As soon as I remove that line, five tests fail.
Here’s an example:
def test_missile_asteroid_scores(self): pos = Vector2(100, 100) asteroid = Asteroid(2, pos) asteroids = [asteroid] missile = Missile.from_ship(pos, Vector2(0, 0)) missiles = [missile] interactor = Interactor(Fleets(asteroids, missiles, , , )) interactor.interact_one_pair(asteroid, missile) assert not missiles assert interactor.score == 20 assert len(asteroids) == 2
Interactor gets the score from its Fleets instance:
class Interactor: @property def score(self): return self.fleets.score
Let’s do this in Fleets:
@property def score(self): keeper = next((k for k in self.others if isinstance(k, ScoreKeeper))) return keeper.score
We’ll fetch the ScoreKeeper and ask it. Slow but it’s only in testing. Twelve tests failing. I suspect none are primed with a ScoreKeeper.
Changing the test to this works:
def test_missile_asteroid_scores(self): pos = Vector2(100, 100) asteroid = Asteroid(2, pos) asteroids = [asteroid] missile = Missile.from_ship(pos, Vector2(0, 0)) missiles = [missile] fleets = Fleets(asteroids, missiles, , , ) fleets.add_scorekeeper(ScoreKeeper()) interactor = Interactor(fleets) interactor.perform_interactions() interactor.perform_interactions() assert not missiles assert interactor.score == 20 assert len(asteroids) == 2
I had to make two changes, one to add the ScoreKeeper, and the other to call
perform_interactions twice, once to get the kill and once to consume the score. I’ll change the others that are similar.
The changes were all trivial, much like the one above, just tedious. Along the way I removed game’s access to the score, as it is not needed. The Fleets instance only provides it for the tests, I’m pretty sure. I’ll look for senders.
I rename the
score properties in Interactor and Fleets to
testing_only_score. That should keep my grubby mitts off of it.
Commit: access to score through Interactor and Fleets is only for testing. Game just lets it happen via Score and ScoreKeeper.
That’s not even what I had in mind when I came here, but I’ll take it.
My original plan was to begin to make the individual collections in Fleets virtual, adding everything to
others and then using list comprehensions to pull out specialized lists, then tick through the use of those to remove everything but tests.
The design mistake was or is leaving access to those collections open at all, but the game was originally built around separate collections, so there are still some places that are looking for them. We’ll need to be sure we root them out.
I am not expecting two more magical moves and everything is suddenly moved into the decentralized mode. A refactoring this major takes time, even in a small program like this one. We have to tease out each thread, one at a time, until we’re done, or close enough.
I say “close enough” intentionally. In a real program being built with other people’s money, we do best to refactor just enough to keep our speed of building new capability high. We are not striving for some kind of perfection, we refactor because when we do the right amount, our speed of developing what people need is maximized. How much is enough? That requires good judgement. How do we get good judgement? By doing things a lot and paying attention. In many cases, that means by doing things wrong and learning why.
For safety, I add a default to the score thing:
@property def testing_only_score(self): keeper = next((k for k in self.others if isinstance(k, ScoreKeeper)), ScoreKeeper()) return keeper.score
Now that next refers to a collection of all the ScoreKeepers from
self.others, plus one fresh one. So we can’t get an exception, only a zero score. In any case, the tests are all doing things right, at least for now.
I think we’ll call this a wrap. Commit: ensure test_only_score always finds a ScoreKeeper.
See you next time! Maybe I’ll do what I plan to do. Maybe.