CODEWITHSUNDEEP
c++
venkysmarty
venkysmarty

Reputation: 11431

deleting memory allocated in static function in C++

I have C++ class as follows

class anotherClass;

class myClass {
   private:
    myClass() {}
    ~myClass() {}

    typedef std::map<string, anotherClass* > stringToClass;
        static stringToClass s_stringToClass;

    public:
         static anotherClass* getStringToclass(string name);

};

in above class for getStringToClass defintion is as follows

anotherClass* myClass::getStringToClass(string name) {
     stringToClass::iterator iter;
     iter = s_stringToClass.find(name);

     if(iter == s_stringToClass.end()) {
          typedef stringToClass::value_type stringToClassPair;
          anotherClass* pothClass = new anotherClass();
          s_stringToClass.insert(stringToClassPair(name, pothClass));
          return pothClass;
     }
     else {
        return iter->second;
     }
}

now my question is we are allocating memory in static function as defined above. How can we delete memory? Can we delete memory in destructor, as destructor is not static?

Thanks for the help.

Upvotes: 2

Views: 3390

Answers (6)

CashCow
CashCow

Reputation: 31435

The collection will automatically clean up but the pointers inside it will not be, so you really have 2 options:

  • Used a collection of shared_ptr which will get cleaned up
  • Use a collection that stores raw pointers but cleans them up

There are classes of the latter type in boost but not sure they have one for maps and I would go for the former.

I would modify the structure too so that instead of using a static function and a static map, I would have a class with a function and a map, and have a static (singleton) instance of that class.

Your function may also be modified to return a shared_ptr but it could still return a regular pointer as the item will remain in the map forever and thus you do not need to worry about it becoming invalidated.

And as it can never be NULL you can even return a reference.

   typedef std::map<string, boost::shared_ptr<anotherClass> > stringToClass;
   anotherClass& myClass::getStringToClass(string const& name)  
   {
       boost::shared_ptr<anotherClass> & value = s_stringToClass[name];
       if( !value )
       {
          value.reset( new anotherClass );
       }
       return *value;
   }

You might consider making it thread-safe too.

Upvotes: 2

Loki Astari
Loki Astari

Reputation: 264331

Problem one:

Functions should never return pointers (unless you really really really have too).
In this case you don't.

A returned pointer has no ownership semantics so it is not clear who the owner of the pointer is (if you don't know who owns the pointer then you don;t know who is reponsable for deleting it).

So either return a refernce or a smart pointer.
In this case a reference. As all the dynamically created objects are being maintained locally.

Since you are obviously new to this. Use a boost::shared_pointer. Technically this is probably not the best one for this situation, but it is the easiest one to just use when learning. I would have a look at the other smart pointers that are available and learn when it is appropriate to use each one.

class anotherClass;

class myClass 
{
   private:
     myClass() {}
    ~myClass() {}

     typedef boost::ptr_map<string, anotherClass > stringToClass;
                               //  ^^^ Note: Not std:: you are not allowed to add class to std::

    static stringToClass s_stringToClass;
    // Ownership now maintained by the map automatically.

    public:
         // Return a reference.
         // We retain ownership inside this class
         static anotherClass& getStringToclass(string name);

};

Code:

anotherClass& myClass::getStringToClass(string name)
{
     stringToClass::iterator iter;
     iter = s_stringToClass.find(name);

     if(iter == s_stringToClass.end())
     {
          s_stringToClass[name] = new anotherClass();
          return s_stringToClass[name];
     }
     else
     {
        return iter->second;
     }
}

Upvotes: 0

Shuo
Shuo

Reputation: 8937

I think in your case, the destructor won't help, because there is no any object of MyClass.

I propose three ways
1. Don't store pointer, store the object itself.
2. Put the delete function into atexit; In your case

class MyClass
{
.....//Your already existing code
static void Destroy()
{
     //iterate s_StringToClass and delete them
}

static void getStringToClass( string name )
{
     struct DestroySetter
     { 
          DestroySetter()
          { 
               atexit( MyClass::Destroy );
          }
     };
     static DestroySetter setter; //Setup the destoyer
     //Your existing code here
}


3. Use smart pointer to manage the resource, shared_ptr is recommended.
Though I put a lot in second way, I suggest the 3rd way.

Upvotes: 0

Benoit Thiery
Benoit Thiery

Reputation: 6387

If you store it in a static variable, I guess you need them till the end of the execution of your process... If not, then you need to add a method to clean this static variable by deleting each element and call it when you don't need it anymore.

You should not do it in the destructor as the static variable is not linked to your class instances.

Upvotes: 0

Kricket
Kricket

Reputation: 4179

Yes, you can delete static variables in the destructor.

That being said, it's not a very good idea. What if you have two instances of your class, both using the static variable, and one gets destroyed? Its destructor would delete the memory, causing problems for the remaining instance of your class.

In addition to your odd usage of static members, it would be wiser to use smart pointers.

Upvotes: 0

Simone
Simone

Reputation: 11797

I'd use some kind of smart pointer such as the ones provided by Boost instead of raw pointers.

Upvotes: 0

Related Questions