[Nepomuk] Nepomuk Core - Questions & Patches

Sebastian Trüg trueg at kde.org
Tue May 18 12:03:15 CEST 2010


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


More information about the Nepomuk mailing list