[Solved] Clean memory allocation
Author
Hi,
I have no idea what the best way to deal with memory allocation is. I'm using vc++ express. I want to be able to,
CDevice * d = createDevice ();
and then,
d->drop ();
and I want all devices to be automatically updated under a single function call run (),
while (run ()) { /*u get the idea*/ }
I've written my attempt at doing this, using c++ new and delete, but it's problematic, because it requires a huge tonne of code devoted to each and every object that I want to have those features.
I can post it if anyone wants, but it's a bit long so I won't post it now.
My method is to keep a list of pointers to all the objects of a type (type is passed using an enum) for every type. That list is updated dynamically so it can have an unlimited number of list items. Every time an object is created, the list is given the pointer and the type to add. Then the list will call the updt() member of every object in the list.
There has to be a better way. A way that doesn't involve object type specific code.
It would be great if I could just have a base class that deals with reference counting / dynamic memory. that way,
class CDevice : public CRef
will be just about sufficient.
Sorry if I'm hard to understand, this is a hard topic to explain.
A link to an article on this very topic would be tops!
Thx
[Edited by - spudmuncher on March 28, 2009 10:14:03 PM]
Create a Device base-class, with a virtual Update() function (and a virtual destructor), and derive your specific devices from that class. Then, have a container of Device pointers and store all devices in it. For updating, you can iterate through the container and call the Update() functions.
As for memory management, if you want to delete a device, you can call delete on a Device pointer, and since the destructor is virtual, the destructor of the actual type will be called (for example, DerivedDevice::~DerivedDevice) - you don't need to do manual bookkeeping for that anymore.
The idea here is hat Device is a generic interface, that is the same for all device types. So, regardless of the actual type, you can all treat them the same. What they do under the hood depends on their actual type (and that's what the virtual keyword is for).
Does that solve your problem?
As for memory management, if you want to delete a device, you can call delete on a Device pointer, and since the destructor is virtual, the destructor of the actual type will be called (for example, DerivedDevice::~DerivedDevice) - you don't need to do manual bookkeeping for that anymore.
The idea here is hat Device is a generic interface, that is the same for all device types. So, regardless of the actual type, you can all treat them the same. What they do under the hood depends on their actual type (and that's what the virtual keyword is for).
Does that solve your problem?
Author
Thanks a lot for your response.
I've tried to make a concept design of how it will work, based on what you said. But I'm new to these advanced C++ concepts because I haven't needed to use them until now, so please let me know if I've done something (or everything) erroneously. Until today I couldn't imagine a possible use for polymorphism (:
[Edited by - spudmuncher on March 15, 2009 5:37:21 AM]
I've tried to make a concept design of how it will work, based on what you said. But I'm new to these advanced C++ concepts because I haven't needed to use them until now, so please let me know if I've done something (or everything) erroneously. Until today I couldn't imagine a possible use for polymorphism (:
// generic object base classclass CRefObject{public: virtual ~CRefObject (); virtual void update ();};// holds a reference to the pointers of all objects (all are the same type, CRefObject)class CPointerContainer{ // list of pointers is dynamically allocated, so it can have unlimited items CRefObject ** pointer; // ** because array of pointers int nPointers;public: ~CPointerContainer () { // destroy all for (int i; i<nPointers; ++i) delete pointer[]; } add (CRefObject * ptr); // won't bother adding insides of this function, basically it adds the given pointer to the container remove (CRefObject * ptr); // again, won't include insides of this function. //It searches for the pointer in the list, and then deletes that pointer, setting it to null clean (); // cleans up the container, slides upper pointers in the list down over null pointers updateAll () { // this would be called in a main loop for (int i; i<nPointers; ++i) { if (pointer != NULL) pointer->updt (); // a null pointer is null because it has been deleted } }} container;virtual void CRefObject:: update (){ // default update action when not specified} // allows derived classes to be cleaned, when the base class is destroyedvirtual CRefObject:: ~CRefObject (){ container.remove (this);}// a couple of derived devicesclass CDeviceA : public CRefObject{public: ~CDeviceA () {}; void update () {/* do the update stuff for this device */};};class CDeviceB : public CRefObject{public: ~CDeviceB () {}; void update () {/* do the update stuff for this device */};};// when I want to create a device I would call this function, which returns a pointer to that deviceCDeviceA * CreateDeviceA (){ CDeviceA * d; d = new CDeviceA; container.add (d); return d;}CDeviceB * CreateDeviceB (){ CDeviceB * d; d = new CDeviceB; container.add (d); return d;}// so in the main loop I can now do thisvoid run (){ CDeviceA * device; device = CreateDeviceA (); container.updateAll (); delete device;}[Edited by - spudmuncher on March 15, 2009 5:37:21 AM]
Quote:
Code: (grrr, neither tabs nor spaces seem to work here to indent stuff )
[code][/code]
or
[source][/source]
1) pure virtual is good for interfaces
class CRefObject{ public: virtual ~CRefObject (){}; virtual void update () = 0; // << pure virtual means you HAVE to implement this (helps prevent typos like "Update()" from working) virtual bool dead() = 0; void deleteMe() { delete this; }};class CDeviceA : public CRefObject{ virtual CDeviceA( /* params */ ) { deviceResources->aquire(); } virtual ~CDeviceA() { deviceResources->release(); } virtual void update() { /* do stuff! */ } virtual bool dead() { /*decide when we are good for removal*/ }}2) std::vector<> is your friend.
std::vector<CRefObject *> updateList;updateList.push_back( new CDeviceA (/* device A params */) );//...while(running()){ std::for_each( updateList.begin(), updateList.end(), std::mem_fun(&CRefObject::update) ); //... std::vector<CRefObject *>::iterator i = std::remove_if(updateList.begin(), updateList.end(), std::mem_fun(&CRefObject::dead); std::for_each(i, updateList.end(), std::mem_fun(&CRefObject::deleteMe); updateList.erase( i, updateList.end() );}std::for_each(updateList.being(), updateList.end(), std::mem_fun(&CRefObject::deleteMe);updateList.clear();3) As in the source above, prefer a "is dead" and centralized removal. It allows you to avoid all the random edge cases that tend to pop up when you add/remove items during the update loop.
4) The CRefDevice should probably inherit from boost::noncopyable under the assumption it aquires some resource that needs removal on delete.
** NOTE!: there could be typos, I typed the code off the top of my head.
[Edited by - KulSeran on March 15, 2009 3:48:55 AM]
Author
Please excuse me for seeming noobish, but....
I didn't really understand what you said KulSeran. Could you try to relate it more specifically to what I wrote. I noticed you used a boost library command, but I'm not using boost. And how does the standard library vector have to do with what I'm doing. Do we have to use external stuff. I guess if it's really worthwhile using std::vector<>, could you please explain how it could fit in with the code I wrote. I'm also a bit confused about the way you changed my code in step 1. Sorry. Thx
I didn't really understand what you said KulSeran. Could you try to relate it more specifically to what I wrote. I noticed you used a boost library command, but I'm not using boost. And how does the standard library vector have to do with what I'm doing. Do we have to use external stuff. I guess if it's really worthwhile using std::vector<>, could you please explain how it could fit in with the code I wrote. I'm also a bit confused about the way you changed my code in step 1. Sorry. Thx
Well, for step 1, I added a "= 0" at the end of the definition of the virtual update function. This tells the compiler that CRefObject does not in fact implement update. So you can't make a CRefObject on its own, and whatever classes inherit from CRefObject are forced to implement update().
For step 2, I didn't use a single boost function. And i replaced your CPointerContainer with std::vector<>.
Your add, remove, and clean functions for that CPointerContainer all require from the looks of it
1) you allocated enough memory to start with
2) you have to iterate over all objects just to find some free space
3) you have to constantly check "is not null? ok do stuff." instead of knowing the container only holds valid pointers.
by replacing your container with the C++ standard container you get:
clarity.
1)speed (no more iterating compacting NULLs, or finding a NULL to use as free space)
2)no need to know how much memory to new/delete for your container.
3)access to the standard library algorithms and utilities.
I re-read your posts, maybe a full example of how I'd do it that you can actually run will help:
* and all of this is just out-of-the-box C++. No boost:: or other libraries.
[Edited by - KulSeran on March 15, 2009 10:31:04 PM]
For step 2, I didn't use a single boost function. And i replaced your CPointerContainer with std::vector<>.
Your add, remove, and clean functions for that CPointerContainer all require from the looks of it
1) you allocated enough memory to start with
2) you have to iterate over all objects just to find some free space
3) you have to constantly check "is not null? ok do stuff." instead of knowing the container only holds valid pointers.
by replacing your container with the C++ standard container you get:
clarity.
1)speed (no more iterating compacting NULLs, or finding a NULL to use as free space)
2)no need to know how much memory to new/delete for your container.
3)access to the standard library algorithms and utilities.
I re-read your posts, maybe a full example of how I'd do it that you can actually run will help:
#include <vector>#include <algorithm>#include <functional>//----------------------------------------------------------------------------// Your device interfaceclass DeviceBase{ public: DeviceBase(){}; virtual ~DeviceBase(){}; virtual void update () = 0; // << pure virtual means you HAVE to implement this (helps prevent typos like "Update()" from working) virtual bool isDead() = 0; void deleteMe() { delete this; }};//----------------------------------------------------------------------------static std::vector<DeviceBase *> updateList;static std::vector<DeviceBase *> removeList;//----------------------------------------------------------------------------// Adds it to the update listvoid AddForUpdate( DeviceBase *pDevice ){ updateList.push_back( pDevice );}//----------------------------------------------------------------------------// Flag it for later removal.// Don't outright remove it, incase this is called durring updatevoid FlagForRemove( DeviceBase *pDevice ){ removeList.push_back( pDevice );}//----------------------------------------------------------------------------// Check if it is in the remove listbool WasRemoved( const DeviceBase *pDevice ){ return std::find( removeList.begin(), removeList.end(), pDevice ) != removeList.end();}void RemoveDeleted( ){ std::vector<DeviceBase *>::iterator i; // Iterator i = std::remove_if(updateList.begin(), updateList.end(), WasRemoved ); // quickly moves all items that "WereRemoved" to the end of the list updateList.erase( i, updateList.end() ); // quickly pops all items past i removeList.clear(); // clear our "deleted already" list}//----------------------------------------------------------------------------// Clear out the listvoid ClearDevices(){ //_______________ // first kill all the stuff that was already deleted RemoveDeleted(); //_______________ // now kill everything else std::for_each(updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::deleteMe)); // destroy the objects updateList.clear(); removeList.clear();}//----------------------------------------------------------------------------// Update loopvoid UpdateLoop( ){ //_______________ // first kill all the deleted stuff RemoveDeleted(); //_______________ // Check for things that died std::vector<DeviceBase *>::iterator i; // Iterator i = std::remove_if(updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::isDead)); std::for_each(i, updateList.end(), std::mem_fun(&DeviceBase::deleteMe)); updateList.erase( i, updateList.end() ); //_______________ // Now update std::for_each( updateList.begin(), updateList.end(), std::mem_fun(&DeviceBase::update) );}//----------------------------------------------------------------------------// Inherited deviceclass DeviceA : public DeviceBase{ public: DeviceA() : _dead( false ) { AddForUpdate( this ); // add for update } virtual ~DeviceA() { FlagForRemove( this ); // add for remove } // virtual update function, so we don't have to know what type of device in the update loop // but the right code gets called virtual void update() { std::cout << "Device A Updating" << std::endl; } virtual bool isDead() { return _dead; } private: bool _dead;};class DeviceB : public DeviceBase{ public: DeviceB() : _dead( false ) { AddForUpdate( this ); // add for update } virtual ~DeviceB() { FlagForRemove( this ); // add for remove } // virtual update function, so we don't have to know what type of device in the update loop // but the right code gets called virtual void update() { std::cout << "Device B Updating" << std::endl; } virtual bool isDead() { return _dead; } private: bool _dead;};//--------------------------------------------------------------// Begin Test Func//--------------------------------------------------------------int main ( u32 argc, char *argv[] ){ //---------------------------------------------------------------------------- // create some devices DeviceBase *newDevice1 = new DeviceA; DeviceBase *newDevice2 = new DeviceA; DeviceBase *newDevice3 = new DeviceB; // Test the update loop UpdateLoop(); // Delete one delete newDevice1; // Test again UpdateLoop(); // Clear out everything ClearDevices(); //---------------------------------------------------------------------------- return 0;}* and all of this is just out-of-the-box C++. No boost:: or other libraries.
[Edited by - KulSeran on March 15, 2009 10:31:04 PM]
Author
Thanks KulSeran for explaining in more detail.
That is far more helpful (:
When I get over this annoying flu I'll have another attempt at it.
Judging by the way certain things looked slightly mysterious, I think I will have to revise my c++ skills.
I'll probably put in a using namespace std;
to make the code look cleaner too (;
A question, are
and
both valid and equivalent
That is far more helpful (:
When I get over this annoying flu I'll have another attempt at it.
Judging by the way certain things looked slightly mysterious, I think I will have to revise my c++ skills.
I'll probably put in a using namespace std;
to make the code look cleaner too (;
A question, are
DeviceBase *newDevice1 = new DeviceA;and
DeviceA *newDevice1 = new DeviceA;both valid and equivalent
Quote:I have no idea what the best way to deal with memory allocation is.
RAII is the way to go in C++. Learn it, it's the basics.
Author
RAII seems very complex.
http://www.codeproject.com/KB/cpp/RAIIFactory.aspx
An OO Variant
Is the only information I've found that is even remotely applicable to my problem. I am struggling to see how to connect it to the work I've done.
So, as I understand it
I would have my generic base class for all devices
Then I would declare the RAII factory
Then I would have all my little devices
Also, is there a way I can manually control the factory life-time, rather than having it only be valid for a single portion of my code (i.e. my devices need to be accessible from a lot of places). I can't seem to find a way to manually remove specific items from the factory either, eg. DeviceA->Drop ();
In the code I wrote, I didn't use a constructor in the base class. And when I create a new DeviceA I would put in new DeviceA (arg1, arg2);. How is this still possible with RAII though.
in MyPolymorphicFactory
what exactly is MyClass in RaiiFactoryImp<MyClass> imp;
Is that some sort of keyword.
http://www.codeproject.com/KB/cpp/RAIIFactory.aspx
An OO Variant
Is the only information I've found that is even remotely applicable to my problem. I am struggling to see how to connect it to the work I've done.
So, as I understand it
I would have my generic base class for all devices
class DeviceBase;Then I would declare the RAII factory
class DeviceFactory;Then I would have all my little devices
class DeviceA : public DeviceBaseclass DeviceB : public DeviceBase;Also, is there a way I can manually control the factory life-time, rather than having it only be valid for a single portion of my code (i.e. my devices need to be accessible from a lot of places). I can't seem to find a way to manually remove specific items from the factory either, eg. DeviceA->Drop ();
In the code I wrote, I didn't use a constructor in the base class. And when I create a new DeviceA I would put in new DeviceA (arg1, arg2);. How is this still possible with RAII though.
in MyPolymorphicFactory
void myFun (int n){ MyPolymorphicFactory factory; for (int i = 0; i < n; ++i) { MyBase* m = factory.create<MyBase> (n); // ... m = factory.create<MyDerived> (n); // ... }what exactly is MyClass in RaiiFactoryImp<MyClass> imp;
Is that some sort of keyword.
I think that article is confusing matters with their 'RAII factory', and the fact that it discourages the use of smart pointers does not speak in it's favor (smart pointers are a fine example of RAII!).
The point of RAII is not to create 'factories' and all that. It's simply to tie resource allocation/deallocation to the lifetime of objects. For your device classes, their constructors create or open the underlying device, and their destructors should ensure that the underlying device is properly disposed of. That's what RAII is about.
So it looks like you're already using RAII for your devices. But, RAII can also be applied to dynamically created objects, by using smart pointers to encapsulate them. If a pointer goes out of scope, it is destroyed, but the object it pointed to is not. A smart pointer makes sure that the object it points to does get destroyed (it does so in it's destructor). There are various types of smart pointers, too, for various kinds of behavior: boost::shared_ptr only removes the wrapped object once all copies of the shared pointer are destroyed, while std::auto_ptr simply removes it's wrapped object when it is destroyed (useful to tie a dynamically allocated object to a scope).
So, to fully automate the memory management, you could use a std::vector of boost::shared_ptr's to Devices:
Upon removing a shared-pointer-to-Device from the vector, if that was the last shared pointer that pointed to that Device, the Device will automatically be deleted.
The point of RAII is not to create 'factories' and all that. It's simply to tie resource allocation/deallocation to the lifetime of objects. For your device classes, their constructors create or open the underlying device, and their destructors should ensure that the underlying device is properly disposed of. That's what RAII is about.
So it looks like you're already using RAII for your devices. But, RAII can also be applied to dynamically created objects, by using smart pointers to encapsulate them. If a pointer goes out of scope, it is destroyed, but the object it pointed to is not. A smart pointer makes sure that the object it points to does get destroyed (it does so in it's destructor). There are various types of smart pointers, too, for various kinds of behavior: boost::shared_ptr only removes the wrapped object once all copies of the shared pointer are destroyed, while std::auto_ptr simply removes it's wrapped object when it is destroyed (useful to tie a dynamically allocated object to a scope).
So, to fully automate the memory management, you could use a std::vector of boost::shared_ptr's to Devices:
std::vector< boost::shared_ptr<Device> > devices;devices.push_back(boost::shared_ptr<Device>(new Device(...)));Upon removing a shared-pointer-to-Device from the vector, if that was the last shared pointer that pointed to that Device, the Device will automatically be deleted.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
