Expert: Ralph McArdell - 2/20/2007

Question
Aloha,
Some history:
Was told that my destructor for a class was causing problems. I destroyed a roguewave vector  that was newed in the constructor.
I have pasted the constructor and destructor
This is okay or should the destructor be blank? If so why?

Thanks
Sheryl

==========================================================
RWTPtrOrderedVector<NewCurrncy>* NewCurrncyList::currncyListPtr_ = 0 ;

//////////////////////////////////////////////////////////////////////////////
//  Method      :   NewCurrncyList
//
//  Description :   Build List
//
//////////////////////////////////////////////////////////////////////////////

//NewCurrncyList::NewCurrncyList(): fatalError(FALSE)
NewCurrncyList::NewCurrncyList(RWDBConnection *aConnection): fatalError(FALSE)
{
   if( currncyListPtr_ == 0 )
   {
       currncyListPtr_ = new RWTPtrOrderedVector<NewCurrncy>(10);

       CurrncyDBT currncyDBT;

       //currncyDBT.Get();
       currncyDBT.Get(aConnection);

       NewCurrncy*       cPtr;

       while( currncyDBT.Next() )
       {
           cPtr  = new NewCurrncy( currncyDBT );

           if (cPtr == NULL)
           {
               fatalError = YES;
               errorMan.Post("NewCurrncyRateList::NewCurrncyRateList",1,"NewCurrncyRate");
               return;
           }

           currncyListPtr_->insert(cPtr);
       }

       cPtr = new NewCurrncy("NUC");
       cPtr->CurrncyDesc("Neutral Unit of Construction");
       cPtr->ToleranceLvl(0.0);
       cPtr->DecimalPlaces(2);

       currncyListPtr_->insert(cPtr); //insert NUC
   }
}



//////////////////////////////////////////////////////////////////////////////
//  Method      :   NewCurrncyList::~NewCurrncyList()
//
//  Description :   clean up
//
//////////////////////////////////////////////////////////////////////////////

NewCurrncyList::~NewCurrncyList()
{
   currncyListPtr_->clearAndDestroy();
   delete currncyListPtr_;
   currncyListPtr_ = NULL;
}
---------

class NewCurrncyList
{
public:
   const static short INVALID_VALUE = -1;


   //NewCurrncyList();
   NewCurrncyList(RWDBConnection *aConnection = NULL);

   ~NewCurrncyList() ;

   short length() const
   {
       return currncyListPtr_->length();
   }

   NewCurrncy*   operator[]( short index )
   {
       return (*currncyListPtr_)[ index ];
   }


   friend ostream& operator <<( ostream& aStream, const NewCurrncyList& aList );
   BOOL         IsFatalError() const
   {
       return(fatalError);
   }

   static short StrToDecPlace(const DString & decStr);
   const NewCurrncy* Find( const DString& currncyCd ) const;
   const short GetDecPlace( const DString& currncyCd ) const;

private:
   // data members

   static RWTPtrOrderedVector<NewCurrncy>* currncyListPtr_ ;
   BOOL         fatalError;

};

#endif  

Answer
You seem to have two points of confusion.

The first is to confuse a class with the objects (or instances) of that class.

The second, following on from the first I think, is that classes are destroyed.

Classes are types. In C++ they have to be defined, static types. That is you define them in the code and all types need to be known at compile time. You cannot dynamically create classes at runtime. Likewise once the compiler has compiled the definition of the class it remains available for the duration of the compilation. In fact for some uses all the compiler needs is the name of the class, not the full definition.

On the other hand a class can have instances. Objects (variables) in C++ have type. That type can be a class type. An object having a specific class type is said to be an instance of that class.

Class members (data and functions) have two storage durations: per instance members and class or static members. Unless otherwise specified a member is per instance. Members prefixed with static are per class or static members.

Each object that is an instance of a class gets one copy of each of that class's instance data member. Calls to instance function members have a this-pointer instance context representing the object on which the instance member function was called.

Instance data members are either implicitly or explicitly initialised by a constructor. They are destroyed implicitly or explicitly in the destructor. Constructors and destructors are special instance member functions.

Static members are different. They exist one per class. So static function members have no this-pointer context because calls are not related to any specific instance of a class. Likewise static data members are not associated with any specific object instance, and so should not in general be initialised in a constructor nor destroyed in a destructor. You can consider such members to be shared by all instances of a class.

In fact static data members have to be explicitly defined exactly once in a program much like global variables. In your example you have placed the line:

RWTPtrOrderedVector<NewCurrncy>* NewCurrncyList::currncyListPtr_ = 0 ;

In the class implementation file.

Now let us consider what happens in your class and its instances with respect to NewCurrncyList::currncyListPtr_.

In the constructor for class instances you define a new RWTPtrOrderedVector<NewCurrncy>. You then delete this new vector in the destructor. However this is a static member and therefore there is only one for the whole class and is effectively shared by all instances. Let us consider what happens in the following code:

   NewCurrncyList a_list;

   // ...

   if ( something_or_other )
   {
       NewCurrncyList another_list;

       // ...

   }

   // ...

   PrintItem( a_list[3] );

When a_list is constructed NewCurrncyList::currncyListPtr_ has a new vector associated with it, and items are inserted into it.

Sometime later, while a_list is still in scope and in use, the Boolean condition something_or_other becomes true, the if-clause is taken and another_list is created. The NewCurrncyList constructor is run again and this creates another new vector and assigns the pointer to it to the single, shared, static NewCurrncyList::currncyListPtr_ member.

Red alert, red alert, red alert.

Problem 1:

The another_list construction has overwritten the pointer placed into the shared NewCurrncyList::currncyListPtr_ by a_list construction. The vector created during a_list construction is therefore no longer referenced (or pointed to) by anything, i.e. you have a memory leak because this object can no longer be deleted as nothing has a pointer to it.

Things progress, another_list is used, and the end of the if-clause is reached. As another_list is local it goes out of scope at this point and is destroyed. The NewCurrncyList destructor is called in the context of another_list, and this causes the vector pointed to by the shared NewCurrncyList::currncyListPtr_ to be deleted. After this point the vector is dead as should not be used again.

Later on an item from a_list is passed to some function PrintItem. The vector referenced by the shared NewCurrncyList::currncyListPtr_ is accessed.

Red alert, red alert, red alert.

Problem 2:

The shared NewCurrncyList::currncyListPtr_ contains a stale pointer that points to a deleted object, the vector used by another_list that was previously deleted.

Basically you have to think harder about what you are doing and how many of each member you actually require. Either currncyListPtr_ should not be static and each NewCurrncyList gets its own vector, or currncyListPtr_ should be static and shared and therefore only needs to be initialised once, not every time a new NewCurrncyList is created. Likewise it should only be deleted once, at the end of the program, if at all (in most modern PC, workstation and server operating systems destroying the program execution process cleans up all resources anyway!).

One way to create the vector only once is to access it through a static member function:

// private static member function
RWTPtrOrderedVector<NewCurrncy> * NewCurrncyList::CurrncyListInstance()
{
   static RWTPtrOrderedVector<NewCurrncy> * instance(0);

   if ( !instance )
   {
       instance = new RWTPtrOrderedVector<NewCurrncy>(10);
   }

  return instance;
}

A simple way to delete the shared vector is to provide a public static function to clean up static data:

// public static member function
NewCurrncyList::StaticCleanup()
{
   delete CurrncyListInstance();
}

This would be called from the end of main for example.

Finally, does the vector really need to be referenced by a pointer and dynamically managed? Could it not just be a RWTPtrOrderedVector<NewCurrncy> object, thus:


   RWTPtrOrderedVector<NewCurrncy> NewCurrncyList::currncyList_(10);

This of course assumes the RWTPtrOrderedVector can be used like this. The benefit is that the whole vector is initialised for you before main is entered and destroyed after main returns, so you do not have to worry about it. The only problem is that you may need to access it from other static data that is also being created and initialised before main is entered and such an item attempts this before NewCurrncyList::currncyList_ has been initialised. To get around this you can again use a static member function instead:

// private static member function
RWTPtrOrderedVector<NewCurrncy> & NewCurrncyList::CurrncyListInstance()
{
   static RWTPtrOrderedVector<NewCurrncy> instance(10);
   return instance;
}

In this case a reference is returned and the function local static is an object not a pointer and so does not require to be dynamically allocated. In this arrangement the static instance object is initialised when NewCurrncyList::CurrncyListInstance is called for the first time. The static instance will be destroyed during program termination.

Oh, and you might like to use a type alias for the rather cumbersome RWTPtrOrderedVector<NewCurrncy>, such as:

   typedef RWTPtrOrderedVector<NewCurrncy> NewCurrncyVector;

Then you can say things like:

   NewCurrncyVector NewCurrncyList::currncyList_(10);

And you might like to consider using currency an Currency in names rather than the oddly compressed currency and Currncy.

I suggest you take time out to review your understanding of C++, its type system, object lifetime rules, class and object model etc. I recommend you also find the time to read some good texts on C++ programming practices. A good start would be Scott Meyers "Effective C++" and then "More Effective C++".