Use Self Documenting Names

You'd think this would be obvious but it's surprising how often I run into source code that doesn't use self documenting names. Many developers will use the shortest names possible. I'm sure plenty of my old code is this way. Some of you may have asthetic reasons for short names. I'd argue asthetics should come after logic and practicality. You're goal should be to write code fast with as few complications and bugs as possible. Code that is easy for others to understand so that they also don't get confused and run into bugs.

Let me attempt to point out some examples of self documenting names and non-self documenting names and the problems they cause.

	void DrawLine(float x, float y, unsigned int color); // <- bad

This looks like a straight forward function except what is color? Is it in the format 0xAARRGGBB or 0xRRGGBBAA or 0xAABBGGRR or maybe something else entirely. Any time wasted looking up what it's supposed to be is one more reason you're going to be sitting at your desk late at night instead of out and about.

void DrawLine(float x, float y, unsigned int colorRGBA); // <- better.

Now it's very clear. Color is 0xRRGGBBAA format so 0xFF0000FF would give you 100% red.

Someone might suggest instead of the type being unsigned int, maybe it should be custom type. That's fine, just name the type something clear. typedef unsigned int ColorRGBA not typedef unsigned int Color

void DrawLine(float xPixels, float yPixels, unsigned int colorRGBA); // <- best

Ah ha!, x and y are in pixels.

How about this one.

void copy(thing* a, thing* b); // <- bad.

does this copy a->b or b<-a?

void copy(thing* a, const thing* b); // <- better.

That helped beacuse obviously we can't copy a->b if b is const but assume the copy function is 100 lines long. Maybe it's copying an array of something and inside there's something like copyarray(a.m_arrayData, b.m_arrayData). Again, looking at that single line I'd have to go reference the definition of a and b to know which side is the source and which is the dest.

void copy(thing* dst, const things* src); // <- best.

This one obviously we copy src->dst, no parsing the code required. And lines like this copyarray(dst.m_arrayData, src.m_arrayData) inside copy will now be readable without having to reference anything else.

Here's some others:

float rotationAmount; // <- bad
float rotationAmountInRadians; // <- better, okay at least we know they are radians
float rotationAmountInRadiansPerSecond; // <- best, now we know 100% what this is.
vector3::velocity; // <- bad
vector3::velocityInMeters; // <- better
vector3::velocityInMetersPerSecond; // <- best
void Update(float elapsedTime); // <- bad
void Update(float elapsedTimeInSeconds); // <- good, ah, it's in seconds.

I heard one programmer object that time wasn't always in seconds because sometimes (bullet time) he slowed down the game. That's pretty much irrelevant though, in his game all calculations are done as though the time passed in is in seconds. It's good to know the time is in seconds, not in frames or some other unit. Name it!

Note that it's okay to shorten or abbreviate. Just be consistent MPerSec or InSec or use a prefix secsElapsed, mpersecVelocity. A very good article that also deals with similar issues this Joel's "Making Wrong Code Look Wrong"

These ideas are not about being a code nazi. They are about being efficient and avoiding bugs. Every one of these has bitten me before. I've had to dig through levels of library code to find out which is source and which is dest in a function. I've spent hours trying to figure out why something did not appear on screen because I was passing radians when the function in question wanted degrees but because the docs just said "rotation" and because the argument was just called "rotation" it didn't even cross my mind until hours of tracking down the issue.

Another common excuse some programmer will bring up is because their editor shows the help or function signatures or has other kinds of helpful features that they don't need to follow these guidelines. That might be true if you're a one man team. On anything larger other programmers are likely to be using different editors which may or may not bring up the same help. I know that in my current project I use both Visual Studio and Visual Slickedit and neither bring up all our help.

So, help out your fellow programmers and automatically document your code by choosing variable names that make the code easier to understand. If I had followed these rules I'd have had to work less, gone home to be with my girlfriend, or moved on to the next feature instead of wasting time figuring out things or tracking down bugs that could be avoided by simply naming things better.


Pass it on

Comments:
Eminently sensible suggestions.

What's your feeling on hungarian, and how far to go with it? I notice you have m_ member prefixes in your examples, but not much else. I find prefixes for basic types very useful, but then we start getting into the debate over whether, for example, a vector or matrix is a basic type.
posted by MarvinSeptember 23, 2007 at 12:54 [ e ]

I consider "m_" part of the same topic. I should have included it because if I see a single line of code like

   position += velocity

I want to know that position and velocity are local variables. If I follow the convension of "m_" is for Member then I know those two are local variables and of course if I see

   m_position += m_velocity

Again I know they are member variables. If I don't follow any convention then my only recourse it to go find the definitions which is a huge waste of my time.

As for hungarian in general. I use "p" for pointer mostly so I know to use -> after it vs if there is no "p" then it's an instance or a reference and I should use "."  Again, I don't want to have to go look. 

    orge.m_hitPoints = 10;
    pOgre->m_hitPoints = 10;

 I use "b" for bool so that I know something should be only true or false.

    active <- Do I set this to 0?, 1?, 20?, ACTIVE_ON?

    bActive <- Ah, it starts with "b" so it's either true or false

That's it. I don't use many of the other "systems hungarian" but I do think that using "apps hungarian" would be a huge plus.

posted by greggmanSeptember 24, 2007 at 13:27 [ e ]
Good tips!

I always try to use Hungarian notation although I don't see much other code doing so lately.
posted by PatrickSeptember 28, 2007 at 2:11 [ e ]
Great tips. I code in ActionScript, but these practices you talk about still apply. It's ridiculous how I come back to a project after a couple months of being side-tracked by something else and I am lost as to how I was handling some things!
posted by RayBOctober 12, 2007 at 12:33 [ e ]

DrawLine2d(...)

is even better in my book

Pet hate: function names with "And" In them....

SubtractHitPoint
sAndCheckIfDead()

ugh.
posted by mrdonutFebruary 11, 2008 at 1:12 [ e ]