Algorithmic Pitfalls

The first group of pitfalls you find here are those hidden in algorithmic choices. These are the kinds of pitfalls you have run into or read about just to find out they exist.

Arrays and Inheritance

Arrays of objects—structures, classes, and base types—are a popular storage structure (refer to Chapter 11, "Storage Structures" ). However, when using arrays in combination with inheritance there is something to watch out for, as shown in Listing 15.1.

Code Listing 15.1. Incorrect Use of Object Arrays

#include <iostream.h>

class AverageJoe
{
   public:
        long socialSecurityNumber;
} ;
class JetSet : public AverageJoe
{
   public:
        long VIP;
} ;

void SetSocialSecurity(AverageJoe* person, long id, long number)
{
     person[id].socialSecurityNumber = number;
}
void main ()
{
    JetSet vips[50];

for(int i = 0; i < 50; i++)
        vips[i].VIP = 1;

    SetSocialSecurity(vips, 1, 0);

    if (1 == vips[0].VIP)
        cout << "VIP!!" << endl;
    else
        cout << "Ordinairy Joe!! 
							
							" << endl;
}

The SetSocialSecurity() function in Listing 15.1 is set up to be able to change the social security numbers in arrays of JetSeters as well as arrays of AverageJoes; this is why it takes a pointer to the base class, AverageJoe. Listing 15.1 compiles just fine, but when SetSocialSecurity() is called upon to change the social security number of the second VIP in the vips array, something unexpected happens. Instead of the social security number of vip[1] changing, the VIP status of vip[0] changes.

// The statement:
SetSocialSecurity(vips, 1, 0);

// Expected result:
vips[1].socialSecurityNumber == 0

// Actual result:
vips[0].VIP == 0

The reason for this becomes apparent when you add the following line to the SetSocialSecurity() function:

// Array element size = 4.
int i = int(&person[1]) - int(&person[0]);

And this line to the main() function:

// Array element size = 8.
i = int(&vips[1]) - int(&vips[0]);

The size of an AverageJoe is 4 bytes (it has one member: a long word). The size of a JetSeter is 8 bytes (it has two members: the long word that is part of the base class and its own long word). This means that the AverageJoe pointer used in SetSocialSecurity() thinks it is iterating over AverageJoe elements and its pointer increases in groups of 4 bytes instead of 8 bytes. person[1] therefore points to the address of the VIP field of the first VIP. The bug in Listing 15.1 can be circumvented by declaring person as a JetSet pointer in SetSocialSecurity(), but then the function can only be used on arrays of JetSeters. If you really need to change JetSeters and AverageJoes with a single function, you should make this function part of the base class:

// New definition of SetSocialSecurity:
void AverageJoe::Set SetSocialSecurity(long number)
{
    socialSecurityNumber = number;
}

// Call to SetSocialSecurity from main:
vips[1].SetSocialSecurity(0);

Do not forget to add the definition of SetSocialSecurity() to the definition of the base class.

Wrap Around of Loop Counters

A danger with counters is that they wrap around when they contain their maximum value and something is added to them (refer to Chapter 6, "The Standard C/C++ Variables," for more details on ranges of types). Adding 1 to a byte that already has the value 255 will cause it to contain the value 0.

// A == 0.
unsigned char a = 255;
a++;

This means you have to think carefully about the variable type you choose for your counters. If, for instance, you want to count the number of IP packets your program receives and you use a short for this, be sure to realize that this counter will reset itself when packet 65536 arrives. When, at a given point, this counter contains the value 5, the number of packets received can be 5, 65541, 131076, and so on. Even sneakier is the following example of a loop that never ends:

unsigned char i;
for(i = 0; i < 256; i++)
{
    cout << "test";
}

Because this loop needs to iterate over 256 elements, an unsigned char was chosen; however, counter i can contain 256 different values (0-255), although its maximum value is of course 255. This means the counter will wrap around before ever reaching the stop clause i >= 256.

Even more deceptive is the following pitfall:

void Cnt(int *a)
{
    char i;
    for(i = 126; i < 200; i++)
    {
        *a += i;
    }
}

Two things go wrong in the function Cnt(): First, at some point in the loop the value of *a will decrease instead of increase. Second, the loop is never-ending. This time the pitfall is caused by the range of the signed variable i. Variable i starts out with the value 126, after the first iteration its value is increased to 127, and after the second iteration its value is suddenly -128. From this point on, the value of i increases again, passing 0 and continuing to 127, then it becomes -128 once more. For more details on signed and unsigned ranges of variables, refer to Chapter 6.

Scope of Loop Counters

In C++ it is no longer necessary to define variables globally or at the very beginning of functions, as is the case in C. Sadly, because of changes in the C++ standard, there is a pitfall associated with this, in particular with defining variables in the headings of for loops. Listing 15.2 shows this pitfall.

Code Listing 15.2. Scoping Pitfall

#include <ostream.h>

#define dataLen 254
#define TERM    13

// Initial Values.
char dataString[dataLen] = { 0,2,3,4,13,8,8,8,8} ;

void main(void)
{
    // find first terminator.
    for(int i = 0; i < dataLen; i++)
    {
        if (TERM == dataString[i])
            break;
    }
    cout << "Terminator at: " << i << endl;
}

On some C++ compilers Listing 15.2 will compile and run just fine. These are the compilers that see the scope of variable i as the body of the function in which it is declared—in this case main(). Consequently, variable i also has a value outside the for loop and can therefore be used to print the position at which the loop was terminated. The construct shown in Listing 15.3 is obviously illegal with these compilers.

Code Listing 15.3. Illegal in Some C++ Compilers

for(int i = 0; i < dataLen/2; i++)
{ 
    /*process first half.*/
}
for(int i = dataLen/2; i < dataLen; i++)
{
    /*process second
							
							
							 half.*/
}

This is because variable i is defined twice.

In the latest version of the C++ standard, the scope of a variable defined in a for heading is limited to the body of that for loop . This means that the following statement in Listing 15.2 will not compile when using a compiler that follows the new standard:

cout << "Terminator at: " << i << endl;

The compiler will complain that variable i is undeclared. The scope of i with these compilers looks like this:

// i undefined
for(int i = 0; i < dataLen; i++)
{
    // i defined.
}
// i undefined.

Listing 15.3, however, is a legal construction with these compilers. In order to write portable/compatible code, it is better to define variables that are used for loop iteration—and variables that need to retain their value outside the loop—as follows:

int i;
for(i = 0; i < dataLen; i++)

Variables for which the scope is the loop body only can of course still be declared inside the loop.

for()
{
    int r, g;

Expression Evaluation and Operator Overloading

Although perhaps not expected, using operator overloading changes the way expressions are evaluated. Listing 15.4 shows how expressions are evaluated normally.

Code Listing 15.4. Normal Expression Evaluation

#include <ostream.h>

class A
{
public:
    int isvalid() {  cout << "Check" << endl; return 0; }
} ;

void main ()
{
    A   Object1, Object2, Object3, Object4;

   if (Object1.isvalid() && Object2.isvalid() &&
        Object3.isvalid() && Object4.isvalid())
    { cout << "Conditions are TRUE" << endl;}
    else
    { cout << "Conditions are FALSE"
							
							 << endl;}
}

In the expression Object1.isvalid() && Object2.isvalid() && Object3.isvalid() && Object4.isvalid(), a call is made to each object's isvalid() method from left to right as long as the call to the preceding object's isvalid() method returned a value greater than zero. The result of Listing 15.4 is thus:

Check
Conditions are FALSE

This is because the call to Object1.isvalid() returns false (0) and therefore there is no need to evaluate the rest of the expression—the result will be false (refer to Chapter 7, "Basic Programming Statements," for more details on expression evaluation). When operator overloading is used, this is no longer the method of evaluation, as Listing 15.5 shows.

Code Listing 15.5. Overloaded Expression Evaluation

#include <ostream.h>

class A
{
public:
    friend int operator&& (const A& left, const A& right)
    { cout << "Check" << endl; return(0);}

    friend int operator&& (const int left, const A& right)
    { cout << "Check" << endl; return(0);}
} ;

void main ()
{
    A   Object1, Object2, Object3, Object4;

    if (Object1 && Object2 && Object3 && Object4)
    { cout << "Conditions are TRUE" << endl;}
    else
    { cout << "Conditions are FALSE"
							
							
							 << endl;}
}

The result of Listing 15.5 is:

Check
Check
Check
Conditions are FALSE

This means that each part of the expression if (Object1 && Object2 && Object3 && Object4) is evaluated. This is important to keep in mind because sometimes you do not want part of an expression evaluated if the preceding part was not true.

Memory Copy of Objects

Using a memory copy method to copy an object can be a very fast way of cloning objects. The standard function memcpy() can be used for this purpose, for instance. There are some pitfalls to watch out for, however. This kind of copy action is known as a shallow copy, which means it does a straightforward copy of all the fields of an object, regardless of its type. Problems arise when one or several of these fields happen to be pointers. A copied object contains the same values as the original, which means that any pointers it contains will point to the exact same addresses as the corresponding pointers in the original object. Listing 15.6 shows how easily this can cause problems.

Code Listing 15.6. Erroneous Shallow Copy

#include <string.h>
#include <memory.h>

class Object
{
public:
    ~Object(){ delete [] data;}
    int  id;
    char name[200];
    char *data;
} ;

Object* CloneObject(Object *in)
{
    Object *p = new Object;

    memcpy(p, in, sizeof(Object));
    return p;
}

void main(void)
{
    char * DataBlock = new char[230];

    Object *object1 = new Object;
    object1->id   = 1;
    object1->data = DataBlock;
    strcpy(object1->name,"NumberOne");

    Object *object2 = CloneObject(object1);
    delete object1;

    // Big Problem:
    delete 
							
							
							object2;
}

In Listing 15.6, object2 is a clone of object1; because the cloning was done through a shallow memory copy, both object1 and object2 have a data pointer that points to DataBlock. Objects have their own destructor which releases the memory pointed to by the data pointer. This means that when object1 is deleted, the memory of DataBlock is released. When object2 is deleted, its destructor will try to release the memory of DataBlock a second time. This, of course, causes strange behavior if not a crash. The use of a shallow copy in this context is obviously flawed, but when using complex and/or third party structures it may not be this obvious. Think of structures that contain a pointer to themselves, and so on.