Source Code Makeover: Demon Kingdom, Part 2

This is a continuation of Part 1. I make a lot of the same changes as before, mostly removing duplicate code.

Single-Item Tuple Gotcha, Getting Rid of Redundant update() Call

Diff of these changes: Fixed single-item tuple syntax error and moved initial calls to update to the AnimatedSprite ctor.

Since the AnimatedSprite class’s update() method is always called after the Monster constructor, so we might as well get rid of that method and put its code directly in the constructor function.

Also, there was a slight bug with my last check-in. In Python, a tuple with just one item in it needs a trailing comma to be recognized as a tuple. (Otherwise it’ll just be seen as a value inside parentheses.) The values in the MONSTER_RATIOS variable need this comma added.

You can see this in the interactive shell:

>>> type( (42, 99) )
<type 'tuple'>
>>> type( (42) )
<type 'int'>
>>> type( (42,) )
<type 'tuple'>

Simplying the “final wave” Variables

Diff of these changes: Got rid of a lot of redundant code by adding a FINAL_WAVE_MONSTERS data structure and a populateFinalWave() function.

There are quite a few “finalWave” variables that have a for loop set up to add various monsters to a list. They look like this:

for i in range(21):
    if i - 4 <= 0:
        type = "bat"
        min = -1
        max = -100
    elif i - 13 <= 0:
        type = "plant"
        min = -20
        max = -70
    elif i - 17 <= 0:
        min = -30
        max = -60
        type = "tree"
    elif i >= 18:
        type = "genie"
        min = -30
        max = -60
    # monster adding code here

I can see what they were doing, but they do it in an overly complicated way. When i is between 0 and 4 then the type is “bat”, else if i is 13 or less (but greater than 4, because of the elif) then the type is “plant”.

But the if statements that check the value of i inside this loop aren’t necessary. It would be better to just have several loops without them, which makes the number of each type of monster much more obvious:

for i in range(5):
    type = 'bat'
    min = -1
    max = -100
    # monster adding code here
for i in range(9):
    type = 'plant'
    min = -20
    max = -70
    # monster adding code here
for i in range(4):
    type = 'tree'
    min = -30
    max = -60
    # monster adding code here
for i in range(3):
    type = 'genie'
    min = -30
    max = -60
    # monster adding code here

This code would be used for each “final wave” variable. But we can reduce this even further by just setting up a function with this code and a data structure for each final wave. The new FINAL_WAVE_MONSTERS variable will hold the data for each level’s final wave of monsters. The new populateFinalWave() function has this code typed out just once.

Put Sequential Variables into a Single List

Diff of these changes: Replaced the finalWaveN variables with a list.

We should put the values of the finalWave1, finalWave2, finalWave3, and so on into a single list varialbe, finalWaves. In this particular case, this doesn’t reduce the amount of code. But if we want to do something with this data in a future change, a list is much easier to work with.

Adding a Function to Replace Redundant Code

Diff of these changes: Got rid of redundant code by adding a populateRandomMonsters() function.

There’s a lot of code in the program that is a copy/pasted form of this:

for i in range(random.randint(10, 20)):
    type = random.choice(MONSTER_RATIOS[0])
    m = Monster(screen, *MONSTER_STATS[type]["image"])
    m.set_rect(random.randint( - 500, -1), random.randint(25, WINDOW_HEIGHT - 70 - SIDEBAR_HEIGHT))
    m.set_speed(MONSTER_STATS[type]["speed"])
    m.set_life(MONSTER_STATS[type]["life"])
    monsters1.add(m)

We can put this code in a single function, and replace all these redundant lines of code with a function call. The new populateRandomMonsters() function does the same thing as these loops.

Put Repeated Code in a Loop

Diff of these changes: Put duplicate code into a loop to populate monster sprite groups.

We see this code repeated for each level:

#Level 1
randomMonsters[0] = populateRandomMonsters(1)
finalWaves[0] = populateFinalWave(1)

By putting this code inside a for loop, we only need it typed out once.

Remove fireballsOff Variable

Diff of these changes: Got rid of fireballsOff SpriteGroup object.

(This check-in was actually a mistake on my part. I originally thought this code was pointlessly duplicated. But the separate fireballs and fireballsOff variables are set up so that the user cannot cast the fireball spell again until the previous fireballs have hit a monster or fallen past the edge of the screen. I’ll correct this in a future update.)

(I make this mistake again in the next check in for the ghosts and whirlwind spells. Diff of these changes: Got rid of whirlwindsOff and ghostsOff SpriteGroup objects as well.)

Regression Fix

Diff of these changes: Fixed a regression with doneText

While changing code, I added a new bug. This type of bug is commonly called a regression. This check-in fixes it.

Putting Duplicate Code for Each Level in a Single Loop

Diff of these changes: Collapsed the code that set the monsters object into a loop.

For each of the six levels there is a wave of monsters followed by a “final wave”. Rather than copy and paste the code for each level, we can put it into a single loop. (After level 3 and 6, we display the “done text” to the user.)

Fixing a Bug and Removing Debug Code

Diff of these changes: Fixed a couple bugs with the levelText and also an exception at the end of the game. and Took out some debug settings and code.

These check-ins were to fix a minor bug, and then a second check-in to remove some temporary debug code I accidentally checked in.

There are a few more changes that we can make in part 3. But so far we’ve reduced a program that had 773 lines of code down to 504 lines of code, which is a huge improvement! A good rule of thumb is that the less code there is, the easier it will be to understand and debug.

(UPDATE: I actually probably won’t be finishing part 3, which wouldn’t have that many novel changes anyway.)

Post a comment.

Switch to our mobile site