==============================================================================
C-Scene Issue #3
10 Steps to Better C++
Chad Loder
==============================================================================

I have put together some suggestions for beginner to intermediate C++ programmers who want to make their applications more robust and spend less time debugging. Also, there is a big difference between classroom code and production code. I have included some suggestions for people who have not yet made the transition between the code they hand in to professors and real-life working code.

  1. Put the constant on the left in a conditional
  2. Wrap your header files in #ifdefs
  3. Handle errors, not bugs
  4. Use asserts in debug builds
  5. Use exceptions
  6. Don't ignore API function return values
  7. Make your code const-correct
  8. Comment before you code
  9. Be consistent
  10. Design a minimal interface

Put the constant on the left in a conditional

We've all experienced bugs like this:
while (continue = TRUE)
{
    // ...this loops forever!
}
This type of problem can be solved by putting the constant on the left, so if you leave out an = in a conditional, you will get a compiler error instead of a program bug (because constants are non lvalues, of course):
while (TRUE = continue)
{
    // compile error!
}

Wrap your header files in #ifdefs

Sometimes many files will include one header file. If you are not very careful, the header file will get parsed more than once, and you will multiply define symbols. Wrap your header files so they will only be included once:
#ifndef __MYFILE_H
#define __MYFILE_H

class MyClass
{
....
};

#endif  // __MYFILE_H

Handle errors, not bugs

There are lots of error conditions that happen in the normal life of a program. For instance, file not found, out of memory, or invalid user input. You should always handle these conditions gracefully (by re-prompting for a filename, by freeing memory or telling the user to quit other applications, or by telling the user there is an error in his input, respectively). However, there are other conditions which are not real error conditions, but are the result of bugs. For example, say you have a routine which copies a string into a buffer, and no one is supposed to pass in a NULL pointer to the routine. You do not want to do something like this:
void CopyString(char* szBuffer, int nBufSize)
{
    if (NULL == szBuffer)
        return;         // quietly fail if NULL pointer
    else
    {
        strncpy(szBuffer, "Hello", nBufSize);
    }
}
Also, don't do something like this (some extremely fault-tolerant systems may be able to justify this, but 99% of applications can't):
void CopyString(char* szBuffer, int nBufSize)
{
    if (NULL == szBuffer)
    {
        cerr << "Error: NULL pointer passed to CopyString" << endl;
    else
    {
        strncpy(szBuffer, "Hello", nBufSize);
    }
}
Instead, do something like this:
void CopyString(char* szBuffer, int nBufSize)
{
    assert(szBuffer);   // complains and aborts in debug builds
    strncpy(szBuffer, "Hello", nBufSize);
};
In a release build, if the user passes in a NULL pointer, this will crash. But rather than think "I never want my application to crash, therefore I will test all pointers for NULL", think "I never want my applications to crash, therefore I will put in asserts and find bugs that result in NULL pointers being passed to routines, so I can fix them before I ship this software".

Use asserts in debug builds

Use asserts liberally in debug builds. You normally don't want to put assert code in release builds, because you don't want the user to see your bug messages. ANSI C (that's ISO for you purists) provides assertion functions in <assert.h> - if the symbol NDEBUG is defined somewhere before <assert.h> is included, then assert() will have no effect. Otherwise, it will print out a diagnostic message and abort the program if its argument evaluates to FALSE. Since 0 == FALSE, you can use assert() on pointers to test them for non-NULL:
void myFunction(char* szFoo)
{
    assert(szFoo);  // same as assert(NULL != szFoo);
}

Use exceptions

Use the try/throw/catch mechanisms in C++ - they are very powerful. Many people implement an exception class, which they use for general error reporting throughout their program. For example:

class ProgramException
{
    // pass in a pointer to string, make sure string still exists when
    // the PrintError() method is called
    ProgramException(const char* const szErrorMsg = NULL)
    {
        if (NULL == szErrorMsg)
            m_szMsg = "Unspecified error";
        else
            m_szMsg = szErrorMsg;
    };

    void PrintError()
    {
        cerr << m_szMsg << endl;
    };
};

void OpenDataFile(const char* const szFileName)
{
    assert(szFileName);
    if (NULL == fopen(szFileName, "r"))
        throw ProgramException("File not found");
    // ...
}

int main(void)
{
    try
    {
        OpenDataFile("foo.dat");
    }
    catch (ProgramException e)
    {
        e.PrintError();
        return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

Don't ignore API function return values

Most API functions will return a particular value which represents an error. You should test for these values every time you call the API function. If you don't want want to clutter your code with error-testing, then wrap the API call in another function (do this when you are thinking about portability, too) which tests the return value and either asserts, handles the problem, or throws an exception. The above example of OpenDataFile is a primitive way of wrapping fopen with error-checking code which throws an exception if fopen fails.

Make your code const-correct

Use const wherever possible. See my article in CScene #2 on const-correctness for an in-depth explanation.

Comment before you code

Before you write a block of code, write the comment for it. This helps you comment as you go and it forces you to think clearly about what the code is going to do (and perhaps more importantly, what the code isn't going to do). If you can't write a clear comment first, you can't write clear code.

Some people tend to restate the code in their comments. Example:

    i++;    // increment i
This is silly - you don't need to rephrase your C++ statements in English next to your code. Assume that whoever is reading your code knows C++. Your comments should be for usage and intent. That is, when you declare a variable, put a comment next to it saying what it's used for. When you have a block of code that does something, comment the entire block of code describing what it does.

Many educational style guides suggest that students put large comment blocks above each function. For example, here is a so-called "good" comment block from a style guide I found on the web:

/*------------------------------------------------ COUNT_SEGMENTS -----
     |   Function COUNT_SEGMENTS
     |
     |  Purpose:  COUNT_SEGMENTS computes the number of segments a 
     |  digital clock will need to display the time given by 
     |  the parameters. It then computes the sum of the digits 
     |  and compares the two totals.  If they match, the success 
     |  is recorded by incrementing the sum 'matches' and by 
     |  displaying the time.
     |          The number of segments a digital clock uses to
     |  display any of the ten numbers 0-9 is stored in the array
     |  'segments'.  The array is indexed by the digit; thus, the
     |  number of segments needed to display a '0' is in element [0].
     |
     |  Parameters:
     |  hour1 (IN) - In a two-digit hour value, this is the leftmost
     |      digit.  Ex:  In the time 12:34, hour1 would hold 1.
     |  hour2 (IN) - In a two-digit hour value, this is the rightmost
     |      digit.  Ex:  In the time 12:34, hour2 would hold 2.
     |  tens (IN) - In a two-digit minute value, this is the leftmost
     |      digit.  Ex:  In the time 12:34, tens would hold 3.
     |  ones (IN) - In a two-digit minute value, this is the rightmost
     |      digit.  Ex:  In the time 12:34, ones would hold 4.
     |  matches (IN/OUT) - The sum of the times the number of segments
     |      equals the sum of the digits.
     |
     |  Returns:  Nothing.  (This is a void function.)
     |
     *-------------------------------------------------------------------*/

void count_segments (int hour1, int hour2, int tens, int ones, int *matches)
{
    // function body...
}
This is ridiculous. You will only find something like this in student assignment programs that are going to be handed in to anal professors. If your comments are more involved than your code, then you double your maintenance costs because every time you change the code, you have to change your comments to match it. It's not worth it - if you think a function needs tons of commenting, chances are you are designing the function improperly.

Be consistent

Be consistent in the way you write your code. Use the same indentation and bracketing style everywhere. If you put the constant on the left in a conditional, do it everywhere. If you assert on your pointers, do it everywhere. Use the same kind of comment style for the same kind of comments. If you are the type to go in for a naming convention (like Hungarian notation), then you have to stick to it everywhere. Don't do int iCount in one place and int nCount in another.

Design a minimal interface

When some people design classes, they tend to go overboard in implementing member functions. Here is an example of an overboard interface:
// integer data container, I have provided every conceivable way of setting the data
class MyClass
{
    // set data from integer
    void SetData(int n);

    // set data from character representation of decimal integer
    void SetData(const char* szDataString);

    // set data from character representation of hex integer
    void SetDataHex(const char* szDataString);

    // set data from double, round it up to nearest int
    void SetDataUp(double d);

    // set data from double, round it down to nearest int
    void SetDataDown(double d);

    // set data from double, floor the double
    void SetDataFloor(double d);

    // set data from user input from stdin
    void SetDataInput();

    // set data from file
    void SetDataFile(const char* szFileName, unsigned long ulOffset);
};
This interface is horrible - it's hard to maintain, and it's prone to bugs. You should only design the most minimal interface first. Later, if you find you're doing something very frequently (like converting hex strings to integers then calling the set function), you can consider implementing a function for it.
This page is Copyright © 1997 By C Scene. All Rights Reserved