Writing code without stupid mistakes
 

The purpose of the following instructions is to help you in your effort to avoid common and stupid, but hard to find and correct, programming errors. All tips are personal opinions so feel obliged to disagree.

This document was originally intended for our ACM programming team. When you write code in a programming contest time is tight and you don’t get a second chance if you plan to be a winner. 

You must write code like crazy and avoid ALL mistakes. 

So some tips should be read with that attitude in mind. However if you follow these tactics in your everyday work you can save yourself lots of trouble and time wasting.

Programming Tips

Use simple algorithms, that are guaranteed to solve the problem in question, even if they are not the optimum or the most elegant solution. Use them even if they are the most stupid solution, provided they work and they are not exponential. You are not competing for algorithm elegance or efficiency. You just need a correct algorithm, and you need it now. The simplest the algorithm, the more the chances are that you will code it correctly with your first shot at it.

This is the most important tip to follow in a programming contest. You don’t have the time to design complex algorithms once you have an algorithm that will do your job. Judging on the size of your input you can implement the stupidest of algorithms and have a working solution in no time. Don’t underestimate today’s CPUs. A for loop of 10 million repetitions will execute in no time. And even if it takes 2 or 3 seconds you needn’t bother. You just want a program that will finish in a couple of seconds. Usually the timeout for solutions is set to 30 seconds or more. Experience shows that if your algorithm takes more than 10 seconds to finish then it is probably exponential and you should do something better.

Obviously this tip should not be followed when writing critical code that needs to be as optimized as possible. 

However in my few years of experience I have only come to meet such critical code in device drivers. We are talking about routines that will execute thousands of time per second. In such a case every instruction counts.

Otherwise it is not worth the while spending 5 hours for a 50% improvement on a routine that takes 2 milliseconds to complete and is called whenever the user presses a big red button. Nobody will ever notice the difference. Only you will know...

 

Write Simple Code. As Simple as Possible.

I wholeheartedly agree with the following statement:

Code is written for programmers not for computers.

I first came upon this in C++ in Action online book. Take the time to read it.

Simple Coding means a lot of things. The following list contains a few tips, but there is a heuristic rule that you can use. Ask yourself : 

"Is there a simpler way to code this ?" 

If there is then use it. In most cases it will make no difference to the compiler. The output code will be the same, so why should you complicate matters? As a developer you already have enough things on your mind to start with. Why making things harder? Complex syntax will only impress college boys and girls, not seasoned professionals.

Remember: Simple Coding aims at eliminating any chance for programming errors that can be avoided.

  • Avoid the usage of the ++ or -- operators inside expressions or function calls. Always use them in a separate instruction. If you do this there is no chance that you introduce an error due to mixing up the difference between post-increment and pre-increment.
    As usual it makes no difference to the assembly code produced by your trusted and hard-working compiler...
  • The above means that you should avoid expressions of the form *p++ even as a single instruction (that is not as a part of a more complicated expression). I haven't written such a thing in years now.
  • Avoid pointer arithmetic syntax. Instead of (p+5) use p[5]. It's easier to chew for mere mortals.
  • Use const as much as you possibly can, and then some more.

    Local Variables: A great number of local variables are initialized with some value and then never get updated. Declare them as const damn it!
    That will protect you from accidentally updating them, will convey to anyone who reads the code that you did not intend to change this variable, and will give the optimizer a valuable additional hint for producing better assembly code.

    Function Parameters: ALL input parameters should be declared const (const & for objects). Make this a rule. NEVER use a function argument as a means to save yourself from an additional local variable. It's stupid and can cause you great pains during debugging.
    Additionally pointer parameters should be declared as const pointers.

    That is instead of coding:
        int f(int *pValue){...}
    type:
        int f(int * const pValue){...}

    This prevents the pValue 'variable' from being updated, but not the integer it points to.

    Member Functions: Any member function that does not update the object's logical state should be declared const. Take the time to learn the meaning of logical constness and what the mutable keyword is really intended to do. Do not use mutable lightheartedly, in the same manner that you would not use a goto lightheartedly. Your code can become a fine mess before you know it, with const objects changing state all the time.
  • Avoid coding something like :

return (x*y)+Func(t)/(1-s);

Instead use :

const double Temp = Func(t);
const double RetVal = (x*y) + Temp/(1-s);
return RetVal;

This way you can check with your debugger what are the values of intermediate expressions.

Similarly:

GetParent()->GetBrother(GetIndex())->DoBrotherThings();

is very inefficient and unhelpful during debugging, because you don't have the return values of GetParent() and GetBrother() handy in order to inspect the state of these objects.

Instead opt for something like:

CParent * const pParent = GetParent();
const int idx = GetIndex();
CBrother * const pBrother = pParent->GetBrother(idx);
pBrother->DoBrotherThings();

It's so much easier to step through such code, much easier to set breakpoints and the compiler doesn't give a darn if it's four lines instead of one. Also note the usage of const pointers.

  • Avoid using the ?= operator. Instead of :

return (((x*8-111)%7)>5) ? y+126 : 238-x;

write :

Temp = ((x*8-111)%7);
if (Temp>5)
    return y+126;
else
    return 238-x; 

This will allow you to step the code and see the execution path that was followed.

If you follow those rules then you eliminate all chances for trivial errors, and if you need to debug the code it will be much easier to do so.

 

When you compare a variable against a constant value, write the constant first in the comparison. This way if you type = instead of == the code will not compile. Don’t think that forgetting the second = cannot happen to you. You don’t need to forget it. It can simply happen from mistyping. Instead of writing (p==NULL) write (NULL==p). If you mistype the second one and write (NULL=p) the compiler will produce an error. You might argue that your ‘clever & advanced’ compiler will issue a warning for if (p=NULL) but when your code already produces 10 warnings that you ignore on each compile because you know they are harmless (that is another bad tactic anyway), you won’t even see the 11th when it happens.

 

As a follow-up, when comparing to zero, or NULL compare to zero or NULL. Write if (X!=0) or if (pMem!=NULL) instead of if (!X) or if (!pMem).
It makes the code more readable.

When I am looking at any code I want to read out the source in my mind and hear what it does. In one case I hear "If X is not zero" and in the second case I hear "If not X".

When rushing through code at 22:30, after several hours of trying to find a killer bug, it is just easier for my brain to hear If X is not zero rather than If not X.

 

When you write logical expressions write all the parentheses. This means that you should write ((5<y) && (10==y)) instead of if (5<y && x-1==y). If by chance you mistype and type a single & instead of two and write (5<y & x-1==y), the expression will be interpreted as ((5<((y&x)-1))==y) which is definitely not what you meant.

Moreover if you always use all parentheses then even if you mistype & instead of && the result will still be correct since the C standard specifies that the result of a logical expression is 0 for failure and 1 for success. Thus ((5<y) & (10==y)) will produce the originally intended result.

 

Follow-up: Never mix arithmetic and binary operators without using parentheses. Binary operators have higher precedence and the result might be undesired.

For example :

x = y<<1 & 0xAF;

is interpreted as

x = y<<(1&0xAF);

NAMING 1 : Don’t use small and similar names for your variables. If you have three pointer variables don’t name them p1, p2 and p3. Use descriptive names. Remember that your task is to write code that when you read it it says what it does. Using names like Index, RightMost, and Retries is much better than i, rm and rt. The time you waste by the extra typing is nothing compared to the gain of having a code that speaks for itself.

In a programming exercise during training for the ACM contest we had a problem where there were rectangles involved and the programmers wanted to implement an intersection operation. Those that used the names Left,Right,Top,Bottom for their struct members were able to program the multiple cases that were involved without an error, while others that chose the names X1,X2,Y1,Y2 (indeed they did !!!) were totally buffled half-way and had to start over again using proper names!

 

NAMING 2 : Use hungarian naming, but to a certain extent. Even if you oppose it (which, whether you like it or not, is a sign of immaturity) it is of immense help.

 

NAMING 3 : Don’t use names like {i,j,k} for loop control variables. Use {I,K,M}. It is very easy to mistake a j for an i when you read code or "copy, paste & change" code, but there is no chance that you mistake I for K or M.

 

NAMING 4 : Use the names I,K,M only for small loops (less than 10-15 lines. For bigger loops use a descriptive name (Index, Count etc) for the control variable. This way you avoid the situation where you write a small loop inside an 100-line loop using the same control variable, which will obviously cause a bug. This can happen very easily because whenever you want a loop you tend to type ‘for (I=0; …)’ taking as granted that I is an available name to use.

 

With regards to indention I propose the following style, since it helps the eye match the start and end braces.

if (0==x)
{
    f(x+1);
    ....
}

A capability of C that is not widely used is the declaration of a variable at the beginning of a block statement (yeah that is true for plain old C dudes!). This helps reduce the possibilities for damaging local or global variables that are in use. For example consider the following code:

void F(int x)

{

char y;

...

if (0 == x)

{

// This is another y. Not a good tactic

// but it is legal and does not cause any trouble.

int y;

y = (x*45) % 7;

f(x+y);

...

}

}

A tactic that can cause errors is initializing variables at their declaration. In this case if you forget to initialize some variable it might be tough to spot inside a bunch of declarations and initializations.

I suggest that you always initialize your variables in separate statements, or that each variable declaration consists of exactly one variable name (so it would be hard to miss the initialization).

 

Write functions not macros. It is very easy to cause trouble with a macro. Here is an example:

#define S(a) a+1

...

x = S(a) & 0xFF00; // This becomes: x = a + (1&0xFF00);

If you insist on using macros then read some basic stuff on how to do them properly. Some basic rules are:

(a) Surround all macro parameters with parentheses, in all places where they appear inside the macro body:

#define S(a) (a)+1

(b) Surround the whole macro definition in parentheses:

#define S(a) ((a)+1)

(c) Use a dummy do\while(0) loop to make your statement-like macro usable as a single statement:

#define CHECK_AND_PROCESS(a, b) \
do { \
  if ((a)!=0) \
    process((b)); \
} while (0)

Now you can write:

if (f(x)>42)
  CHECK_AND_PROCESS(x, f(x+1));
else
  CHECK_AND_PROCESS(f(x-1), x);

and not sweat it at all!

Moreover... make damn sure you don't type a space after the line continuation backslash in a multiline macro... You can't imagine the fun that compilers have with that. You will get an amazing avalanche of errors, one of which will be something like 'unrecognized escape sequence'.

You bet that you will miss that in the mess that will be spilled in front of you, and you can also bet that it's rather hard for your eyes to spot a space after the backslash... Give it a shot. Try any C/C++ compiler you can get your hands on. They all as helpful as a Japanese web page to a blind Southafrican!

Note: Some code editors have a mode in which the display whitespace. That can be useful sometimes...