PDA

View Full Version : Destructor not running



ShamusVW
22nd August 2016, 08:38
I have created a class for logging of errors into a text file, and the logging works, however I'm not seeing the destructor ever run.
I am testing this by having the destructor write to the file.
Can someone explain when the destructor is actually supposed to be called?

Thanks, this is my full code. If you can suggest a better way to code, please let me know, as I don't like doing things that work, but are not best coding practices.


#include "errorlogging.h"

ErrorLogging::ErrorLogging()
{
QString fileName = "c:/ProductionDB/SIMSErrors.daq";
m_File = new QFile(fileName);
m_File->open(QIODevice::Append | QIODevice::Text);
}

void ErrorLogging::logError(QString timeStamp, QString classInstance, QString errorMsg)
{
QTextStream logStream(m_File);
logStream << timeStamp << classInstance << errorMsg << "\n";
}

ErrorLogging::~ErrorLogging()
{
QTextStream logStream(m_File);
logStream << "File closing...";
m_File->close();
}

anda_skoa
22nd August 2016, 09:53
This looks ok, aside from m_File never being deleted, but is definitely not your full code.
This lacks the place where the class is actually instantiated.

In general:
- if you create the object of a class on the stack, it will be deleted when the current scope ends, e.g. the function scope.
- if you create the object of a class on the heap, then you need to explicitly delete it

For the latter case there are also classes called smart pointers which can bind heap allocated objects to scopes or getting refernce counted handles, etc.

Cheers,
_

ShamusVW
22nd August 2016, 10:24
Hi anda_skoa, you seem to be the one who most often gives advice, thank you.

In another class I have the following where I instantiate this error logging class, along with a test of the logError method.
After this, I only close the application, believing[/hoping!] that all destructors then get called automatically.


m_ErrorLog = new ErrorLogging;
m_ErrorLog->logError("1", "2", "3");


Also, would this be the new destructor then, in order to delete the m_File variable?


ErrorLogging::~ErrorLogging()
{
QTextStream logStream(m_File);
logStream << "File closing...\n";
m_File->close();
delete m_File;
}

I declare m_File as


private:
QFile *m_File;

Lesiok
22nd August 2016, 10:38
After this, I only close the application, believing[/hoping!] that all destructors then get called automatically.Very bad practice. By the way I think you should read about the singleton.

ShamusVW
22nd August 2016, 11:38
I know about the Singleton, but not sure how it applies to this?
Basically to create a single instance of an object, and if further objects get created, they revert to the original one created.
Don't ask me to program it though, because in that case I WILL have to go and read up on it, it has been a long while.

My understanding with destructors is that they got called automatically when the object went out of scope as andy_skoa mentioned, however, when the application terminates, why is the error logger class's destructor not called?
Surely the object gets destroyed when the application ends, and hence the destructor should be called?

Lesiok
22nd August 2016, 12:00
According to me this is a typical situation when you use a singleton - the entire application should use one instance of logger.

Object constructed by new is never going "out of scope". Objects are terms of language not an application. You can mix languages in one application. The application manages the memory, not objects.

anda_skoa
22nd August 2016, 12:10
After this, I only close the application, believing[/hoping!] that all destructors then get called automatically.

When the process ends, the operating system will reclaim things like memory, open files, etc.,
But once the program has ended it can't execute any code anymore (well, otherwise it would not have ended, would it :) )
So only destructors of objects get executed that get destroyed while the program is still executing.




m_ErrorLog = new ErorLogging;
m_ErrorLog->logError("1", "2", "3");


This creates an object on the heap. If there is no matching delete, then the object remains indefinitely.



Also, would this be the new destructor then, in order to delete the m_File variable?

Yes.
You could even drop the call to close(), the QFile destructor will do this anyway.

Cheers,
_

ShamusVW
22nd August 2016, 12:11
Ok, thanks, I understand what you are meaning to use it for.
The way I was planning to do it is pass the object along to each class/object as needed.
So if I created a new object from the class where I first created the logger object (i.e. m_ErrorLog), I would then use: anotherObject = new className anotherObject(m_ErrorLog); <--- passing the object between other objects created
Then in the newObject, I will then just reference the logging method (logError), I wouldn't instantiate a new logger object again.

What I have read is that there is debate on the use of the Singleton class, and that there are "better" ways to do things.
I don't have enough experience and knowledge to have an opinion, but it seemed to me that if people were throwing into question using it, then I should look at other ways to do it.
Is my passing of objects around bad?

anda_skoa
22nd August 2016, 12:25
My understanding with destructors is that they got called automatically when the object went out of scope as andy_skoa mentioned

Yep, they do.



however, when the application terminates, why is the error logger class's destructor not called?

Because the object that goes out of scope in your case is the pointer to your error logger, not the error logger itself.
The pointer is a primitive datatype (like an int), it doesn't have any destructor (it isn't a class, just an address value).



Surely the object gets destroyed when the application ends, and hence the destructor should be called?
No. Your application code never destroys the object, i.e. you never call delete on the pointer to the object.

Added after 10 minutes:


The way I was planning to do it is pass the object along to each class/object as needed.
So if I created a new object from the class where I first created the logger object (i.e. m_ErrorLog), I would then use: anotherObject = new className anotherObject(m_ErrorLog); <--- passing the object between other objects created
Then in the newObject, I will then just reference the logging method (logError), I wouldn't instantiate a new logger object again.

Sensible approach.



What I have read is that there is debate on the use of the Singleton class, and that there are "better" ways to do things.
I don't have enough experience and knowledge to have an opinion, but it seemed to me that if people were throwing into question using it, then I should look at other ways to do it.
Is my passing of objects around bad?

A singleton has a couple of properties/behaviors and it is sometimes used for only one of them and not always is that a good reason.

One thing a singleton is good at is the use case where there absolutely can only be one instance.
A singleton class' constructor is usually private, only the instance() method can create an object of the class.

Sometimes,however, singletons are just used like global variables, so the usual arguments against global variables apply.

Loggers often use it for a combination, i.e. both for the convenience of global access as well as having a central object that all logging passes through.
But I've also seen loggers being passed through, e.g. when applications have multuple loggers or components are allowed to create their own, local, loggers.

One thing with singletons is that their objects are often never explicitly destroyed or such explicit destruction needs to be added to the end of main()
(while instantiation is often implicit in the first call to the instance access function)

Cheers,
_

ShamusVW
23rd August 2016, 06:30
Thank you Lesiok & Anda_skoa for your feedback on destructors, object passing and the Singleton pattern.
It has given me direction to investigate this further, as I need to brush up on destructors and when they run, and also deleting of objects.