elgolondrino
elgolondrino

Reputation: 665

Dynamic members allocation in qt

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

Answers (3)

hluk
hluk

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

bash.d
bash.d

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

Lohrun
Lohrun

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

Related Questions