Hi Vishesh, the patch looks pretty good. I have only two comments: 1. I think having ref(), deref(), and cnt() inline is still a good idea since they are called fairly often. 2. Don't we need to protect m_resources via a QMutex? [3. Wouldn't shoudBeDeleted() make more sense in ResourceManagerPrivate?]
As for ResourceManagerPrivate: this is just how it is done in KDE: it is a convention to name private classes that way. You can look in KDE and Qt code all over the place and find these private classes. Their main purpose is to keep as much member variables and methods out of the main class as possible to make it possible to add and remove those without breaking binary compatibility. If you are confused as to why ResourceData is not called ResourcePrivate: it is another concept - shared data. I will apply the patch and test it. Cheers, Sebastian On 05/16/2010 04:11 PM, Vishesh Handa wrote: > Cleaned up the code a little bit. > - fixed a small bug in Resource::~Resource(). (my fault) > - m_resources is now managed along with reference counting. > - Added a function ResourceData::shouldBeDeleted() to avoid duplication > of code. > > I think the shouldBeDeleted() function should be in another patch. > > Btw, this code still hasn't been extensively tested. I'll do the testing > later on. I just though I should share the patch so that I can get your > comments. > > - Vishesh Handa _______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
