Monochrome Programming: Variable naming, object orientated programming, code design

PROGRAMMING PART 1

Monochrome has been easily my biggest project, with 5376 total lines of code, not counting spaces or small header files, with 596 of them being comments. With that being said, it shouldn’t come as any surprise that some mistakes were made in the design, or architecture of the code. Luckily, Monochrome was both small enough that poorly written or structured code, from a non-functional point of view, was not crippling, and yet Monochrome was also big enough that I was punished for writing bad code enough to want to improve.

I read an article, written back in January of 2013 and read by me prior to any work on the game, praising the Doom 3 code base. The author, Shawn McGrath, claimed that Doom 3 was “the cleanest and nicest looking code [he’d] ever seen.” He defined “nice looking”, or “beautiful” code as:

1) Code should be locally coherent and single-functioned: One function should do exactly one thing. It should be clear about what it’s doing

2) Local code should explain, or at least hint at the overall system design.

3) Code should be self-documenting. Comments should be avoided whenever possible. Comments duplicate work when both writing and reading code. If you need to comment something to make it understandable it should probably be rewritten.

Frankly, I don’t think that was as useful to me as it ought to have been. While I mostly succeeded but sometimes failed at 1), and the codebase was so small that 2) was basically impossible not to achieve, the knowledge of 3) lead me to under-commenting my codebase.

I’m not saying that the article is wrong, I’m just saying that my mistakes were usually in a different area. I found that my mistakes were in wastefully using vertical and horizontal space, and mis-organizing code in a variety of different ways.

VARIABLE NAMES

Before I get into any of the interesting parts of the code, the number one mistake I made, is a very simple and correctable problem: I used variable names that were way too long. More specifically, I used incorrect variable name lengths. Variable names should be clear over all else if they are by themselves, which can mean writing things such as:

bool allEnemiesDead;     //we have a much better chance of understanding this then
bool dead;                          //what does this do?
bool enDd;                         //again, don’t know what this does.
bool allEnDd;                   //better, but still not as easily and quickly readable as the above.

So I think it’s very clear that, while brevity is a massively useful skill, any given person will be much more articulate when given more characters to write. With that in mind it’s easy to make the mistake of having all variables with long names. While all variables should hopefully be titled clearly, I made the mistake of giving structure variable names that are simply way too long. Case in point:

//these two lines are really long to type, but a lot better than…
trs.clBx[i].x=trPs.sets[nmTrs-1].ars[trAr].ps[i].x;
trs.clBx[i].y=trPs.sets[nmTrs-1].ars[trAr].ps[i].y;

//these two lines aren’t even two lines anymore in WordPress.
turrets.collisionBox[i].xPosition=turretPositions.sets[numberOfTurrets-1].arrangement[turretArrangementIndice].position[i].xPosition;
turrets.collisionBox[i].yPosition=turretPositions.sets[numberOfTurrets-1].arrangement[turretArrangementIndice].position[i].yPosition;

These lines are so long, that on my 1920×1200 monitor, at any reasonable font size using notepad, sitting less than two and a half feet from my monitor, I don’t have enough room to comment beside them. This is a huge problem, because horizontal readability is a massive problem when writing code. I have my debugging information off to the right, while my file management is off to the left. It ends up meaning that I have about two thirds to only a half of the actual width of my monitor to write code in, and when I have to scroll horizontally to read each line code readability falls off a cliff, no matter how clear each variables name is. On the other hand, if I break one line into two lines, vertically, then I lose vertical real estate, and I can’t read as much code at once as I could earlier.It’s not uncommon to have a line of code that looks like this:

if (blts.newBolt(trs.clBx[i].x, trs.clBx[i].y, boltSpeed, 128, PC, trs.angle[i], true)){
//do corresponding things
}

//Which is already hard to read, but much better than:
if(bolts.fireNewBolt(turrets.collisionBox[i].xPosition, turrets.collisionBox[i].yPosition, speedOfBoltsInUnits, PlayerCharacter, turrets.angleFacing[i], true)){
//do thing
}

This is massively wasteful, almost to a ridiculous extent. Even with auto complete, this is still going to be a pain to type, and isn’t even that much more readable. This is far from the worst example though. Let’s say we wanted to see if the player character is close enough to a bomb that it’s within the radius of triggering the bomb, and so the bomb should explode.

if(((PC.x-Bmb[i].x)*(PC.x-Bmb[i].x)+(PC.y-Bmb[i].y)*(PC.y-Bmb[i].y))>BOMB_RAD*BOMB_RAD){
//have bomb explode
}

Imagine instead:
if(((PlayerCharacter.xPosition-bombs[i].xPosition)*(PlayerCharacter.xPosition-bombs[i].xPosition)+(PlayerCharacter.yPosition-bombs[i].yPosition)*(PlayerCharacter.yPosition-bombs[i].yPosition))>BOMB_TRIGGER_DISTANCE*BOMB_TRIGGER_DISTANCE){
//have bomb explode.
}

I almost can’t begin to explain how ugly this code is, and this is just variable names. I could have made the situation even worse by having the player character struct hold a struct that holds it’s positions, such as ps, or position. I could also have made both lines of code much more readable by simply doing:

float xDif=PC.x-bmb[i].x;
float yDif=PC.y-bmb[i].y;
if((xDif*xDif+yDif*yDif)>BOMB_RAD*BOMB_RAD{
//do thing
}

…which probably does more to increase readability than the variable names themselves. This has the drawback of taking up extra vertical real estate, which might, but probably won’t, offset the gains in immediate readability, and lesser chance of bugs. For an example of the latter, I accidentally wrote (…PC.y-bmb[i].x…), instead of (…PC.y-bmb[i].y…), during the writing of this example, which is a horrible bug that will compile perfectly but give the wrong result. Much easier to make small errors like that in the longer code.
Luckily, there aren’t any examples of horribly unclear or long variable names left in the codebase, at least that I could see.

I’ve created a nice rule for myself:
If it’s by itself: Make it clear, try to make it short.
If it’s a struct variable: Make it short, try to make it clear.

SCATTERED CODE

The astute reader might have started to catch me writing struct instead of class, despite programming in C++. This is because, with the exception of enumerated values, and c++ text file handling, I ended up using straight C written in C++. This is because using methods was easily the number one way I ended up having code scattered all over the place. When I started creating only structs, and started treating my classes like structs, not adding any more methods, I started to regain some hold on my code. A struct, in C and C++ is a type of variable that stores a bunch of other variables. Imagine if we have a hunter enemy type. That’s a collection of data such as: floats for position, velocity, collision boxes, animation frames, images, sounds, etcetera. We might have some code that looks like this:

struct Hunter{
float xPos, yPos, xVel, yVel;
SDL_Rect colBox;
float angle;
…and many others.
};

In C, or really C++ without classes, if we wanted to move any individual hunter we might have to do something like this:

void MoveHunter(Hunter* pHnt){
//code for moving hunter. Obviously not real.
pHnt->xPos=50;
pHnt->yPos=50;
}

In the part of the code where we want to do this we would have something like this:

MovePC(&PC);
MoveHunter(&hnt);    //probably an array to iterate over all of them, but that’s irrelevant.
MoveSniper(&snp);

However, in C++ we can have methods, which is a fancy way of saying that we include the functions that do things to the data, into the data structure itself. What that would look like in practice is this:

class Hunter{
private:
float xPos, yPos, xVel, yVel;
SDL_Rect colBox;
float angle;
…and many others.
public:
void moveHunter();
};
void Hunter::moveHunter(){
//code for moving hunter. Obviously not real.
xPos=50;
yPos=50;
}

Then in the part of code where we are calling this it would look like this:

//As an aside, we probably would just name the methods .move() because it’s redundant.
PC.move();
hnt.moveHunter();
snp.move();

I think the benefits of this are massively oversold. What we have done is make our code slightly more self-contained. There is really very little difference between the two. I fell into the trap of having a ton of methods because I created the classes as controllers for ALL of the enemies of that type. So the Hunters (notice the s) class stored arrays containing all the information that the Hunters data type called for. To rewrite the example I used above it would look like this:

class Hunters{
float xPos[8], yPos[8], xVel[8], yVel[8];
SDL_Rect colBox[8];
float angle[8];
…and many others.
void moveHunters();
};

My idea was to have one object that was a controller for each enemy type. I did this mostly because I thought it would be more CPU cache friendly, for reasons that I don’t want to go into, but also because I thought I could have self-contained enemy types that I could just plug and play into all the different game modes. Here’s the problem, enemy types are not self-contained units. We often times want to do thing TO the enemies as opposed to having them do it to themselves. That sounds vague, so here’s an example:

I thought that I could use the constructors, methods that initialize a class (fancy way of saying create a new object of that type to specific parameters), as a way to spawn new hunters. This might have worked, except that we would need to create a new object every new level, or room, and destroy the old one, which seems wasteful. We also might want to keep certain parts of the state around from the old object and how are we going to put that into the new object?An easier example to understand is setting up the turrets for each new scenario. The code looks like this:

for(int i=0; i<nmTrs; i++){
trs.clBx[i].x=trPs.sets[nmTrs-1].ars[trAr].ps[i].x;
trs.clBx[i].y=trPs.sets[nmTrs-1].ars[trAr].ps[i].y;
trs.rBx[i].x=dt.xPush+trs.clBx[i].x*dt.scl;
trs.rBx[i].y=trs.clBx[i].y*dt.scl;
trs.lastFire[i]=eventTimer.getTicks()+i*FRAME_TIME*8;
}
//wordpress has problems with tabs. Ignore the formatting.

All this is doing is resetting the firing times for the turrets, and placing the turrets in the right spot. That’s because that’s all we need to do. This simple task is a nightmare to do in a class system where all the data is private. We need to either create 5 .setData(value) methods, or we need to create a method that takes in all the right data at one time and sets these values for us. If we wanted to do that we could just as easily do the same thing to a structure, and really, what’s the point of object orientated programming here? When the data inside a class is getting set by a bunch of different outside events, why bother hiding it in the first place?

I feel that if object orientated programming was instead called self-contained programming I wouldn’t have made so many of the mistakes I made. The phrase object-orientated leads me to think of physical objects, which is not really the point.There were some things that worked just fine, such as movement for the characters, which took a few parameters and then could be shut off just fine, but for the most part, it just didn’t make sense to pretend that the structures I had were self contained units.

New rule for myself:Don’t use classes, or if you do, DO NOT automatically make all the variables private.

This entry was posted in Monochrome, Programming and tagged , , , , , . Bookmark the permalink.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s