[Nepomuk] Nepomuk Core - Questions & Patches

[Vishesh Handa]

Hey Sebastian!

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.
>

Done!


> 2. Don't we need to protect m_resources via a QMutex?
>

I'm currently blissfully unaware about concurrent programming. I assure you
I'm trying to learn, but till then could you take care of it?


> [3. Wouldn't shoudBeDeleted() make more sense in ResourceManagerPrivate?]
>
> :-/ It checks if a ResourceData should be deleted. I considered making it
static but this approach had less clutter. Could you please explain why it
should be 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.
>

Yea. I know. The "pimpl" idiom. :)

I will apply the patch and test it.
>
> Okay. I'll do the same.

- Vishesh Handa


> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100518/15584ae4/attachment-0001.htm 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: removeProxyPatch3.diff
Type: text/x-patch
Size: 12702 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/nepomuk/attachments/20100518/15584ae4/attachment-0001.diff