============================================================================== 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.
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! }
#ifndef __MYFILE_H #define __MYFILE_H class MyClass { .... }; #endif // __MYFILE_H
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".
void myFunction(char* szFoo) { assert(szFoo); // same as assert(NULL != szFoo); }
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; }
Some people tend to restate the code in their comments. Example:
i++; // increment iThis 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.
// 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.