In this blog post, I’m taking a game off of Pygame.org and going through it to make it more readable and extend its functionality. This is an intermediate level tutorial and assumes some familiarity with Python. This can be pretty helpful if you know programming basics but want to know, “How can I write better code?" (And because someone always brings it up, I have this disclaimer: Of course, these changes are my own subjective idea of "better" and not necessarily changes that another developer would make.)

Demon Kingdom by Logi540 is a defense game where monsters walk from the left side of the screen. The player needs to attack them by clicking on them, and can pick up gems to use for spells. If any monster reaches the right side of the screen, the player loses. Download source code.

The main theme of my changes is:

• Removing duplicate code. Duplicate code is bad because if you have to change it (to add features or fix bugs) you may forget to change it in every duplicated place.
• Remove magic numbers. Magic numbers are hard-coded integer values which are bad because they don't describe what they represent. A better alternative is to use a constant instead (these are the ALL-CAPS variables).
• Put sequential variables in lists and use loops. Instead of having variables named like spam1, spam2, spam3, and so on, it's better to have a single list variable, and have a loop run code on each item in the list. This usually leads to a decrease in duplicate code.
• Get rid of unneeded variables. Removing code reaps tons of benefits: there's less code a programmer has to read and understand, less code that a programmer has to look through when debugging, and less code (usually) means less bugs and "moving parts" that could break.

Here's the inital check in of all the files. The file I'll be modifying is named demonkingdom_makeover.py. This program requires Pygame to be installed to run. I recommend downloading the game, playing it a couple times, and looking through the source code before continuing with this article. For each section, open up the diff link to see what the exact changes I made were.

Remove Dependency on easygui

Diff of these changes: Removed easygui requirement and changed image & sound file paths

The first change I'm going to make is to get rid of the use of the easygui module. The game uses this just to display a message box that displays some credits. Without this call to easygui.msgbox(), the fewer additional modules the user will need to run this game (and the more likely people will play it).

I've also put all the image files under their own folder named "images", so I'll have to change the lines of code that load these to include the new path.

Add Spaces for Readability

Diff of these changes: Renamed SpellIcon class. Added spaces after +, -, and * operators.

As a general cleanup of the code to make it more readable, I prefer a single space after commas in the source code so that it is not bunched together. Using the find-and-replace feature in my text editor, I replace all commas with a comma and space. Diff of these changes: Added spaces after all the commas

I do some more cleanup by adding spaces around the +, -, and * operators in the source code, as well as capitalizing the spellIcon class. It is a common convention in programming to have class names begin with a capital letter.

Improve the Background Class

Diff of these changes: Improved the Background class.

Next, I make some improvements to the Background class. This class loads and stores all the background images as pygame.Surface objects. Previously the single Background object would be passed an image object with its changeBackground() method. In order to put all the background-loading code in the same place, I had this take place in the constructor method of the class.

The Background constructor is now passed a list of background images, which is loads and stores in its imageList member. A single list variable is always preferable to variables like back2, back3, back4, and so on. Now when changeBackground() is called, we simply pass it the index of the image it should be changed to.

Also, the rect member is updated based on the image when changeBackground() is called. Previously the code assumed that all the background images were the same size and used the rect from the first background for all the backgrounds.

Replace Magic Numbers with Constants

Diff of these changes: Improved Sidebar class. Created numGems global. Added size constants. Renamed Logo class.

Next I want to get rid of the magic numbers used in the source code. These a numbers that appear in the source code that do not have explanations, which can make it hard to figure out what they are used for. Instead we will replace them with constant variables with descriptive names. These include the MAP_WIDTH, MAP_HEIGHT, SIDEBAR_HEIGHT, WINDOW_WIDTH, and WINDOW_HEIGHT variables.

I also rename some variables to be more descriptive (such as changing gems to numGemsCache) and created a new numGems global variable. Normally I don't advocate using global variables, but in this case the code had placed the gems member in the Sidebar class. This is odd because the Sidebar class should deal with the user interface and displaying game info, not tracking the state of the player. Also, only one Sidebar object is created, effectively making it a global variable anyway. This change also lets me get rid of the addGems() method from the Sidebar class.

I replace several calls to the SpellIcon constructor with a loop (lines 34 to 41 of the original file), and capitalize the name of the Logo class.

Get Rid of the Unneeded load() Method

Diff of these changes: Got rid of first_frame member. Renamed MySprite class. Renamed spellEffect class. Got rid of the load() methods in the AnimatedSprite classes.

Originally each monster is an object of the Monster class, which is a child class of the MySprite class, which itself is a child of Pygame's pygame.sprite.Sprite class. This is an odd choice to make because a monster is not a sprite image, but rather several things that can include sprite images. However, I've found it difficult to break out this inheritance structure, so instead I just renamed MySprite to the more descriptive name AnimatedSprite.

I renamed the color-related variables (white, lightGrey, etc.) to capitalized names to show that they are constants.

Since the load() method in the AnimatedSprite class was always called directly after the constructor, I saw no reason why there should be a separate load() method and moved its code into the constructor.

Also, the first_frame member variable never has a value other than 0, so I got rid of it and simply used 0 in its place. (Though it could be argued that that is a magic number and should be replaced with a constant.)

I capitalized the name of the SpellEffect class.

Make __str__() More Debug-Friendly

Diff of these changes: Added labels to the __str__() method of AnimatedSprite.

Next, the __str__() method of a class is what is called when a string value representation of the object is needed. This is useful to print out debugging information about an object, but the AnimatedSprite class's __str__() method only printed out values with labeling what they are. The developer would have to look back at the source code to find what each number stood for. I've added labels to these values.

Add Spacing to Make MONSTER_STATS More Readable

Diff of these changes: Improved readability of the monster stats, renamed stats constant.

The stats global variable holds all the details about the image, size, life, and speed attributes of each type of monster. I renamed this variable to MONSTER_STATS (to denote it as a constant) and also added some spacing and newlines to make it easier to read in the source code.

Change Lists to Tuples, Make Monster Constructor Call More Compact

Diff of these changes: Some variable renames, switch lists to tuples in MONSTER_STATS, and simplified Monster ctor calls.

Next I renamed the rect member with the more descriptive frame_rect name (it is the rect object for the frame of the monster's image. I replaced the lists in the MONSTER_STATS global with tuples since these values do not change. A tuple is immutable; and using a tuple conveys to other programmers that these values never change when the program runs.

I also call the Monster constructor using an asterisk with MONSTER_STATS[type]["image"]. The asterisk passes all of the items in the MONSTER_STATS[type]["image"] tuple as individual arguments to the Monster constructor. This is much more compact than calling Monster(screen, MONSTER_STATS[type]["image"][0], MONSTER_STATS[type]["image"][1], MONSTER_STATS[type]["image"][2], MONSTER_STATS[type]["image"][3]).

Don't Use Python's Existing Names, Don't Use Sequential Variable Names

Diff of these changes: Put variables into lists.

The use of time as a name used in different parts of the source code is a poor choice because there is a module already named time as part of the Python standard library. You shouldn't create your own variables with the names of things that exist as a part of Python, because it can confuse other developers and lead to bugs in your code. (For example, if we also imported the time module in this program, we might accidentally use the wrong time at different places in the code.) I renamed it to current_time. Diff of these changes: Renamed time to current_time.

The source code has several sequential variable names such as level1Text, level2Text, level3Text, and so on. These would be much more manageable in a single list variable. This way we can put the code that uses them into a much more compact loop, rather than a lot of copy/pasted code.

Also, I renamed the sword variable to swordSound, which is more descriptive of what it contains and is consistent with the other sound-related variable names.

Shorten "== False" and "== True" Code

Diff of these changes: Shortened the == False and == True expressions.

Instead of having expressions that check for equality with True or False, it is more concise to leave this out. I rewrote code like while introdone == False: to the more simple while not introdone:.

Put Copy/Pasted Code Into Loops

Diff of these changes: Put spell casting code in a loop.

I rename the spells variable to SPELL_STATS to show that it is a constant. (Oddly again, this variable is in the Sidebar class. I'll move it out in a future change.)

Instead of having a separate if or elif block for each of the three spells, I made a loop that goes through the SPELL_STATS variable and generically handles each spell. This will make it much easier to add spells to the game in the future.

(The numGems = 90 change was mistakenly checked in. I had set it to this to test out the game.)

Loops are always preferable to copy/pasting code, because if you have to make a change to it later, you only have to change the code in one place instead of every place it was copy/pasted.

Get Rid of Unneeded mouseLastDown Variable

Diff of these changes: let MOUSEBUTTONDOWN event handle clicking code. Got rid of mouseLastDown variable.

Instead of checking the pygame.mouse.get_pressed() function to see if the mouse button is currently pressed, we can just examine the MOUSEBUTTONDOWN event that gets generated with each mouse click. This lets us get rid of the extra mouseLastDown variable, and the fewer variables our program has, the easier it is to understand.

Put Monster Ratios into a MONSTER_RATIOS Variable

Diff of these changes: Moved the monster proportion data to a new MONSTER_RATIOS variable.

Each level has a group of randomly selected monsters from code like type = random.choice(["bat", "bat", "bat", "bat", "plant", "plant", "plant", "orc", "orc", "orc2", "slime"]). The random.choice() function will randomly pick a monster from the list. Instead of having these values hard-coded across the source code, we can put them all into a single MONSTER_RATIOS variable.

That's it for Part 1. In the next part, I'll continue to remove duplicate code and make some readability changes.