Skip to main content
GameDev.net gamedev.net
🔒 Locked

Destructor vs Cleanup()

Started by simpler Oct 12, 2012 at 12:59 PM 28 replies 9.7k views
Original Post
simpler
simpler
Hey I have bumped into a lot of code that uses a Cleanup() member function for their classes instead of using the destructor. I have found myself starting to mix both alternatives which feels bad and I'd rather want to be consistent. What are the pros and cons? What's your opinion?

EDIT: The same question goes for the constructor and Init().
swiftcoder
swiftcoder
There is almost never a good reason for a separate 'cleanup' function in C++. You are stuck with that sort of thing in a garbage collected language (i.e C#'s IDisposable), but the careful use of RAII renders manual collection superfluous in C++.
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
IggyZuk
IggyZuk
I usually use a Cleanup method in languages that don't use destructors.
But with C++ it's better to just stick with constructors and destructors, what swiftcoder said.
Start by doing what is necessary; then do what is possible; and suddenly you are doing the impossible.
SiCrane
SiCrane
There are occasionally reasons to separate construction and initialization or destruction and clean up, most of which involve the fact that constructors and destructors are limited in their ability to convey error information. On platforms with poor exception handling, such as some consoles and embedded systems, or if you need to do a significant amount of interop with languages that don't understand C++ exceptions, then you may be in a situation where you want to do the heavy lifting in separate functions.
frob
frob
It also makes sense for reusable objects, especially heavyweight objects that are expensive to create and destroy, but cheap to recycle.

An example of reusable objects would be the C++ file streams. You can use it once by create, open, close, then destroy; or reuse with create, open, close, open, close, open, close, ... etc., then destroy.
swiftcoder
swiftcoder

It also makes sense for reusable objects, especially heavyweight objects that are expensive to create and destroy, but cheap to recycle.

Let me rephrase my earlier statement to read: "One should never aim to design code that requires a cleanup function in C++".

If you have an object which should be reused, or which lifetime should exceed its use, then consider allowing it to provide an RAII-compliant 'handle' object, which encapsulates the short-term operation.

An example of reusable objects would be the C++ file streams. You can use it once by create, open, close, then destroy; or reuse with create, open, close, open, close, open, close, ... etc., then destroy.[/quote]
Are fstreams really that heavyweight? Glancing at my local header files it looks like a mutex, a buffer, an underlying stream, and a handful of state variables. It seems to me that the cost of actually locking the mutex, and invoking an open syscall should dominate the construction costs of allocating the buffer.
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
frob
frob

An example of reusable objects would be the C++ file streams. You can use it once by create, open, close, then destroy; or reuse with create, open, close, open, close, open, close, ... etc., then destroy.

Are fstreams really that heavyweight? Glancing at my local header files it looks like a mutex, a buffer, an underlying stream, and a handful of state variables. It seems to me that the cost of actually locking the mutex, and invoking an open syscall should dominate the construction costs of allocating the buffer.
[/quote]Please reread my post. No, they are not heavyweight. They are reusable.
Bregma
Bregma

Please reread my post. No, they are not heavyweight. They are reusable.

Sure, they're reusable, but the heavy cost is in the open operation, not the object creation operation (except when the object creation includes the open operation).

Reusing an fstream is like reusing an integer variable: the cost to maintainabilty heavily outweighs the runtime cost savings.
Stephen M. Webb
Professional Free Software Developer
frob
frob

[quote name='frob' timestamp='1350061504' post='4989518']
Please reread my post. No, they are not heavyweight. They are reusable.

Sure, they're reusable, but the heavy cost is in the open operation, not the object creation operation (except when the object creation includes the open operation).

Reusing an fstream is like reusing an integer variable: the cost to maintainabilty heavily outweighs the runtime cost savings.
[/quote]
The point was not about reuse cost.

The point was that sometimes a "reset this object" function is a good thing.
Servant of the Lord
Servant of the Lord
Another (unfortunate) reason for needing a Cleanup() or Initialize() function is from poor third-party API designs that you have to work around in your own code. dry.png


It also makes sense for reusable objects, especially heavyweight objects that are expensive to create and destroy, but cheap to recycle.

An example of reusable objects would be the C++ file streams. You can use it once by create, open, close, then destroy; or reuse with create, open, close, open, close, open, close, ... etc., then destroy.

And conveniently, (some of) the constructors call open(), and the destructors call close() for you, so actual use can look like: create, destroy, or create, open, destroy, or create, close, open, destroy. Very nice design - very flexible.

Another benefit of that, is it makes the destructor and constructor implementation cleaner, especially when you have multiple constructors (and don't yet have constructor chaining).
MyFile::MyFile()
{

}

MyFile::MyFile(filename)
{
this->Open(filename);
}

My::MyFile(data)
{
this->OpenFromData(data);
}

MyFile::Open(filename)
{
data = ::LoadFileData(filename);
this->OpenFromData(data);
}

MyFile::OpenFromData(data)
{
//actual loading from data...
}


Edit: Bah, code boxes messing up. rolleyes.gif
swiftcoder
swiftcoder

The point was that sometimes a "reset this object" function is a good thing.

And that's precisely the point I am questioning.

If the object had steep construction cost, then by all means, (carefully) provide reset/reuse functionality. But that is the only case in which I see it as a good thing.
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
Cornstalks
Cornstalks
A manual cleanup() function is okay so long as it also automatically calls this in the destructor. If I'm not required to call it for resources to be released and they will be done so automatically when the object is destroyed, I have no problem with it, and in some cases it might be nice to be able to reset/reuse the object, in which case the cleanup() function is ok.

For example, in C++11 I can do [font=courier new,courier,monospace]vector.clear(); vector.shrink_to_fit();[/font] to destroy all the objects in the vector and clear its allocated memory. However, I'm not required to do this, as the destructor will do the same thing for me too.
lride
lride
If you have global objects that requires other objects to be initialized or destroyed before initializing or destroying themselves, init() cleanUp() comes in handy.

[source lang="cpp"]MemoryManager memoryManager; //memoryManager must be initialized before other objects
RenderManager renderManager; //but c++ doesn't guarantee that memoryManager gets
AIManager aiManager; //initialized in the order it appears in the source code
[/source]
In this case we can make constructors and destructors do nothing and put init() and cleanUp() instead to ensure the order.
[source lang="cpp"]MemoryManager memoryManager;
RenderManager renderManager;
AIManager aiManager;// constructors do nothing here

int main()
{
memoryManager.init();
renderManager.init();
aiManager.init();

//Game Running...............
//game over-shut things down
aiManager.cleanUp();
renderManager.cleanUp();
memoryManager.cleanUp();
return 0;
}[/source]
An invisible text.
Daaark
Daaark
Re: Constructor vs Init()

Because sometimes they are 2 different concepts.

A lot of my game objects just call a Reset()/Init() function in their constructor because I will reset them to an initial state over the course of their lifetime. A lot of times because there is a fixed number of them, and they disappear and reappear on a frequent basis. By calling Reset, I effectively create a new one, instead of allocating and de-allocating a new instance of an object for nothing. Also, sometimes I need to clear out references and other things that are no longer valid. A re-used object doesn't have a last attacker or a target yet.

I've also used it for particles. When a particle dies, it gets Reset(). Which means it returns to the origin, and then re-assigns itself some random properties as if it was a new instance.
swiftcoder
swiftcoder

If you have global objects that requires other objects to be initialized or destroyed before initializing or destroying themselves, init() cleanUp() comes in handy.

I have two issues with this:
a) You should most likely not be using global variables (and as a bonus, the order of destruction for member variables is well defined).
b) You should not have hidden dependencies between objects. If object B requires object A to be allocated first, then for goodness sake enforce that invariant in code.
Tristam MacDonald. Ex-BigTech Software Engineer. Future farmer. [https://trist.am]
SiCrane
SiCrane

[source lang="cpp"]MemoryManager memoryManager; //memoryManager must be initialized before other objects
RenderManager renderManager; //but c++ doesn't guarantee that memoryManager gets
AIManager aiManager; //initialized in the order it appears in the source code
[/source]

Actually, C++ does guarantee that global variables defined in the same translation unit are constructed in order of definition.
Bregma
Bregma
I've been developing software a long time. A long time. I've seen a lot of trends come and go, and been amazed by and adopted the better ideas over the decades.

One of the ones that works well, really well, is the idea of class invariants. They're facts about objects of the class that are always true (except perhaps during the execution of a member function body). To have class invariants, you need to construct your objects in the constructor, and destroy them in the destructor. Period.

Now, I'm sure all you all are really clever and probably write genius-level code, so you don't have to be able to reason about code you're written 6 weeks after you wrote it. Me, I appreciate the fact that I can rely on those good old class invariants to hold true whenever I see an object lying around. So, go ahead and partially construct objects and then initialize them later, and maybe clean them up and leave them lying around before you destroy them, and don't bother with validity checks on each and every access because, providence knows, you never write buggy code. But when you've just spent 12 hours trying to track down that crasher the day before release and you finally find the problem, do think of me. Don't think this is a real life problem? Consider a std::istream, how it does not have a proper class invariant, and how many questions in these very forums are spawned by people not properly checking the object state before trying to use it.

Of course, there's nothing wrong with a reset function. Just so long as those class invariants don't get violated.
Stephen M. Webb
Professional Free Software Developer
L. Spiro
L. Spiro
frob and Cornstalks gave the correct answers.

Here is an excerpt from my engine. A basic std::vector class:
The constructor:
LSE_CALL CSVectorCrtp() :
m_tLen( 0 ),
m_tAllocated( 0 ),
m_ptData( NULL ) {
}

The Reset function:
/**
* Reset the list completely. Frees all memory.
*/
LSVOID LSE_CALL Reset() {
for ( LSINT32 I = static_cast<LSINT32>(m_tLen); --I >= 0; ) {
Destroy( static_cast<_tDataType>(I) );
}
m_paDefaultAllocator->Free( m_ptData );
m_ptData = NULL;
m_tLen = m_tAllocated = 0;
}

The destructor:
LSE_CALL ~CSVectorCrtp() {
Reset();
}



There is no harm done in the fact that it can easily be reused. Anyone who argues otherwise should swiftly be ignored.
It can obviously be used on a normal one-off basis, as is standard, but it is such a general-purpose class that who is to say it will never be useful to reuse the same object?
I personally do it all the time. It is just another option for our convenience and control.

Speaking of control:
/**
* Reset the list without deallocating the whole buffer.
*/
LSVOID LSE_CALL ResetNoDealloc() {
for ( LSINT32 I = static_cast<LSINT32>(m_tLen); --I >= 0; ) {
Destroy( static_cast<_tDataType>(I) );
}
m_tLen = 0;
}

While there are ways to control std::vector to achieve the same results, they are fairly non-intuitive. It is a lot easier to just use Reset() and ResetNoDealloc(),which have obvious meanings.


Reusable classes are never a bad thing. They are just another layer of options, and that is always a good thing.


L. Spiro
I restore Nintendo 64 video-game OST’s into HD! https://www.youtube.com/channel/UCCtX_wedtZ5BoyQBXEhnVZw/playlists?view=1&sort=lad&flow=grid
JohnnyCode
JohnnyCode
yes, from C# object gets anownced that it would like to be freed (By OS manager GC). You then perform alll logic to free resources the object posesss. Like sicrane said, study cli/c++

in c++ destrocturos are not necesary, nor any good logic, from C#, managememet of memory yes

You can still design your logic to not need destructors, but your logic must be freed and managed well

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.