Source Code Makeover: Square Shooter, Part 1

When there is nearly identical code, I like to make the spacing match up so it easier to compare them with each other. In this case, you can see that they are all the color variables are tuples of three integers.

BLACK = ( 0, 0, 0)

GREEN = ( 0, 204, 0)

RED = (255, 0, 0)

SILVER = (204, 204, 204)

WHITE = (255, 255, 255)

Note that powerup_types is both a global list defined on line 35 and as a static list in the GameWorld class on line 138. But if you keep grepping for “powerup_types” you’ll see it’s only used once in the program. Let’s get rid of this duplication by deleting lines 35 and 138, and just putting a tuple with the three strings in the one place where it is used.

If we used this list of powerups in more than one place, then putting it in a variable would make sense. But since it’s only used in one place, we shouldn’t take the extra step of putting it into a variable.

And since this list of three strings is never going to be modified, let’s use a tuple instead of a list because tuples are immutable data types.

I’m going to go ahead and commit my changes now with the log message, “Aligned the color constants and got rid of the powerup_types variable.”

Getting rid of magic numbers

(These changes can be seen in the github history.)

The scale_and_round() function can be improved. 480 is a magic number. Magic numbers are bad. The number 480 means something to the program, but as a human looking at it the number doesn’t tell me anything (and there’s no comments to explain them either). Using a small program called PixelRuler I can confirm that the game map (not the entire window) is 480 x 480 pixels. That’s what 480 in scale_and_round() must refer to. Let’s change those 480 ints to global variables MAP_WIDTH and MAP_HEIGHT.




self.bglayer.fill(GREEN, (MAP_WIDTH, 0, 160, MAP_HEIGHT))

There are some 480 integer values in the program that don’t refer to a width or height so much as a generic size. Since the map is a square, it doesn’t matter. But if we ever want to make the map dimensions flexible, we should put this in a variable as well. Let’s add a MAP_SIZE global constant as well, and just set it to the larger of the MAP_WIDTH and MAP_HEIGHT constants using max().


I also notice on line 522 that the midbottom of some text for the pause screen is placed at the X coordinate 240 (in the middle of the screen’s width). This is another magic number that we can get rid of. Let’s replace it with a MAP_HALF_WIDTH constant (and you can see that 120 and 360 are also used, so we can add MAP_QUARTER_WIDTH and MAP_THREE_QUARTER_WIDTH constants too, as well as constants for height). We don’t use all these values, but for completeness let’s just add them anyway in case one day we do change the program to use them.

But what we do need is a WINDOW_WIDTH and WINDOW_HEIGHT for the pygame.display.set_mode() call. Even though WINDOW_HEIGHT and MAP_HEIGHT are both 480, they don’t describe the same thing. This is why we use two separate constants for them. (Think of it like this: We would want two constants ONE_DOZEN and NUMBER_OF_DUCKLINGS even though they may both contain be 12.)

Note that the MAP_SIZE, MAP_QUARTER_WIDTH, MAP_HALF_WIDTH, and MAP_THREE_QUARTER_WIDTH are not directly set with values, but calculated from MAP_WIDTH and MAP_HEIGHT. This way, if we are playing around with these values we only have to update MAP_WIDTH and MAP_HEIGHT, and all the others are automatically updated.

model.shoot_at(x / 480.0, y / 480.0)

model.thrust_at(x / 480.0, y / 480.0)

Page 2 of 8 | Previous page | Next page