Reputation: 665
I'm confused with where to dynamically allocate (QFile* and QTextStream*) in the constructor (as it is illustrated below) or in the method itself, if a method GetCurrentStream() ought to change the value *stream.
header file
class QLogger {
public:
explicit QLogger();
~QLogger();
QTextStream& GetCurrenStream();
private:
QFile *file;
QTextStream *stream;
};
and in related .cpp
QLogger::QLogger() {
file = new QFile;
stream = new QTextStream;
}
~QLogger() {
delete file;
delete stream;
}
QTextStream& GetCurrenStream() {
...
return *stream;
}
And where to release the storage in the destructor?
Upvotes: 0
Views: 574
Reputation: 6018
Consider using smart pointers to manage your dynamically allocated objects.
In Qt you can use QScopedPointer
(in C++11 there is also std::unique_ptr
).
(Also as mentioned in comments above: Q* names are basically reserved by Qt stuff.)
class Logger {
public:
Logger();
QTextStream& getCurrenStream();
private:
QScopedPointer<QFile> file;
QScopedPointer<QTextStream> stream;
};
Logger::Logger()
: file(new QFile)
, stream(new QTextStream)
{
}
QTextStream& getCurrenStream() {
// ...
return stream.data(); // stream.get() with std::unique_ptr
}
Memory is automatically deallocated by smart pointers (with QSharedPointer
and std::shared_ptr
there is additionally reference counting so memory is not deallocated while there exist a copy of the shared pointer).
Upvotes: 1
Reputation: 13207
If you are able to provide your object with all the necessary data the moment it will be constructed, you should put all initialization in the constructor, for it creates the actual object. This way, you make sure, that your object will be ready when you use it.
If you are not using any special pattern, your getters should not create the objects they return, but rather a reference, as you did in GetCurrentStream()
.
Secondly, with the destructor, this is the actual place to deallocate memory from allocated objects. If you have deallocation-processes that might be dangerous, you should provide an extra method for this, because if something fails in the destructor, you'll experience a memory-leak. Also, never throw exceptions in a destructor!
If you need to change a pointer to an object (like in GetCurrentStream()
, you should rather provide a different method for changing the stream and handle necessary deallocation in this very method).
Also remember to return a reference to a pointer like
return *stream; //return reference to actual object, not the pointer!
All in all it looks fine, what you did there.
Upvotes: 2
Reputation: 6732
What you did seems about right. You allocate objects in the constructor, and delete them in the destructor.
But, it should not build as is, since GetCurrentStream()
is supposed to return a reference to the stream object and you are currently returning a pointer.
Something like the code sample below is probably what is missing.
QTextStream& GetCurrenStream() {
...
return *stream;
}
Upvotes: 1