Monday, May 17, 2010

Lugaru - Part 4

I'm excited to announce that by my fourth post on Lugaru, I have doubled my subscribers! Now in addition to my sister, I have the creator of Lugaru following. Hopefully my mother will subscribe and triple my readership in a single month...

Well, after spending a weekend tinkering with the code, I finally have a running version of Lugaru for Windows. Having a running application will allow me to do two things:
  1. Experience Lugaru as a player with the benefit of stopping the game any any point I choose to look at the game's state in code.
  2. Verify any changes I make don't cause unintended consequences in the game. Remember, refactoring is about improving the design of software without changing its outward functionality.
After poking around, I've fallen in love with the file containing the game's logic of weapons. In the CodeCity, the yellow building below is the "Weapons" class.



Why start with weapons? Well, think about how you can take a human being and decompose it into mind, body, and soul. You can then decompose the body into systems (nervous, digestive, circulatory, etc). Decompose circulatory to heart, arteries, veins... all the way down to the point of cells and molecules and atoms. Object Oriented programming does the same thing - takes a complex problem and breaks into into manageable pieces while also defining the interactions between those pieces. If you were going to redo the human body (lawl) you'd either want to start by identifying problems at the macro- or micro- level. "Weapons" to me is the micro of Lugaru.

Okay, so enough intro to programming. Let's dive in to Weapons...

Unfortunately, we break our necks because we hit the shallows! Let's talk about the file name "Weapons." It's named that because it contains the class "Weapons."
Design Smell #1: Class name is plural

A golden rule of OO programming is that classes represent a single representation of something. If you need multiples of that something, you instantiate multiple objects of that class. As it stands, a single instance of "Weapons" handles 30 weapon objects. When we get further into the code, we'll see the issues that creates.

Code Smell #1: Favoring #defines for constants

Some quantities in a game never change, for example the downward pull of gravity as 9.81. That quantity could be reproduced all over the game's files. If you want to change that to a more precise number or change it from metric to imperial units, however, you'd have to go hunting for every single place you typed "9.81". Heaven help you if you over-look a place! That's why you use a constant. A constant says, "I am defining this word to represent this quantity." You can then use that word all over your game, and if you need to change the quantity down the road, you only have to change it in one place, the word doesn't change!

In C++ there are multiple ways to create a constant. The Wolfire team chose to use #defines.
#define max_weapons 30
#define max_weaponinstances 20

#define knife 1
#define sword 2
#define staff 3
Using #defines for constants are not good for several reasons. First, a #define is a pre-processor directive. It's a very low-level command that literally replaces every instance of the token with the quantity before processing the file. If something about the quantity causing a compile error, there's no way for the compiler to acknowledge the #define macro. Let's pretend a clumsy programmer fat-fingered some extra characters into the knife macro:
#define knife 1dkf
The next time the program was compiled, he'd get a slew of errors (I got 145 when I tried it.) The first three errors I got were against the following line of code:
if(type==knife)
and the errors were:
  1. syntax error: 'bad suffix on number' [ed. I see no number here]
  2. syntax error: missing ')' before identifier 'dkf' [ed. I see no dkf here]
  3. syntax error: ')' [ed. I don't even know what this means!]
Yes, I realize the IDE you're developing in might give you some tips, but there are other problems with this approach. Second, #defines do not offer compile-time type checking - a big feature of C++. Since #defines are token replacements, the compiler has no problem with the following:
int weapon_type = max_weaponinstances;
weapon_type = rabbittype;
weapon_type = tracheotomy;
All of which are macros in Lugaru that are associated with integers, none of which are associated with weapon types. This, coincidentally is why using
static const int knife = 1;
Is better than a #define but still has problems. That's why I like enumerations. They allow you to group constants and get compile-time type checking. The weapon constants we saw above are re-written as:
enum Weapon_type
{
knife = 1,
sword = 2,
staff = 3
};

And that means the 'type' attribute in the Weapons class can go from an integer to a Weapon_type. That gives a reader a much clearer idea about what the valid values of 'type' are.

Of course, the penultimate solution is to replace these type codes with polymorphic classes, but refactoring is about going from 'bad' to 'better' one step at a time (as opposed to 'bad' to 'perfect' in one giant leap.) Are there better ways to define constants? What advantages do the alternatives have over my proposal?

No comments: