PDA

View Full Version : 100% correct usage of QSharedData



coder2012
20th May 2012, 01:00
Hi all,

Just tried to create my first data class using Qt's implicit sharing, but the documentation out there is misleading in multiple cases.
My header currently looks like this:


class MyData;

class MyBase
{
public:
MyBase(const QString& name, const QString& address);

const QString& getName() const;
const QString& getAddress() const;

private:
QSharedDataPointer<MyData> d;
};

class MyData : public QSharedData
{
public:
QString name;
QString address;
};

Source file:


MyBase::MyBase(const QString& name, const QString& address)
{
d = new MyData();
d->name = name;
d->address = address;
}

const QString& getName() const
{
return d->name;
}

const QString& getAddress() const
{
return d->address;
}


This already compiles, but misses some parts depending on what documentation you read. What comes to mind:
Empty destructor ~MyBase() {}
Copy constructor MyBase(const MyBase& other) : d(other.d) {}
Assignment operator operator=(const MyBase& other) {if(this != &other) d = other.d; return *this;}
Constructors, destructors and assignment operators in MyData as well

Is any of them strictly required or does my implementation also work well without them? Maybe some are implicitly compiler-generated, I have no idea.
I've read that this also depends on whether you declare MyData inside the MyBase header file. I definitely want to do this, so please tell me if this is the reason I can leave out these things.

Thank you!

amleto
20th May 2012, 01:40
mydata doesn't need anything added. mybase needs a dtor otherwise you have memory leak.

ChrisW67
20th May 2012, 04:40
The declaration and implementation of the private class MyData would typically both be in the cpp file. This hides the internal structures from those that do not need to know about them. The header file declares MyBase and only provides a forward declaration of MyData in order to make a pointer to one.

coder2012
20th May 2012, 10:48
mybase needs a dtor otherwise you have memory leak.
How does an empty destructor prevent a memory leak?


The declaration and implementation of the private class MyData would typically both be in the cpp file. This hides the internal structures from those that do not need to know about them.
As I'm developing a standalone application, which isn't mixed with code I don't know about, I don't care who can see the internal structures.

But can anyone confirm that I'm not missing any constructors or overloaded operators for a functioning shared container class?
If not, what are these additional copy constructors or assignment operators for?

ChrisW67
21st May 2012, 01:14
How does an empty destructor prevent a memory leak?
It doesn't (at least a non-virtual destructor). Destruction must deallocate resources allocated during construction. If you were not using the smart pointer then the allocated MyData instance would be a memory leak requiring a non-empty destructor. In this instance the QSharedDataPointer will delete the MyData instance if necessary, and the d object will always be destroyed with your MyBase object.

If you intend that MyBase can be inherited then it should have an empty virtual destructor so that memory is not leaked by deleting a derived object through a MyBase*.



As I'm developing a standalone application, which isn't mixed with code I don't know about, I don't care who can see the internal structures.

Fine. Your call.


But can anyone confirm that I'm not missing any constructors or overloaded operators for a functioning shared container class?
If not, what are these additional copy constructors or assignment operators for?
You need a copy constructor and assignment operator if the default C++ behaviour is improper or if you wish to prohibit these actions by making them private. QSharedDataPointer, through its copy constructor/assignment operator, takes care of a great deal for you during a default copy, so you more than likely do not need a copy constructor. Disabling copy in a shared data class does not immediately strike me as sensible; you could not share the data.