[Nepomuk] Nepomuk Core - Questions & Patches

[Sebastian Trüg]

Hi Vishesh,

please find attached an updated version of the patch. There is still at
least one problem: run the resourcetest and see it crashing. This is due
to determineUri deleting "this". determineUri() is called all over the
place. I think the one solution is to move that call into Resource as I
already did with store().
The other thing is the comment in ResourceData::remove(). We need to
make sure that is still working.

Cheers,
Sebastian

On 05/18/2010 01:46 PM, Vishesh Handa wrote:
> 
> 
> On Tue, May 18, 2010 at 4:07 PM, Sebastian Trüg <trueg at kde.org
> <mailto:trueg at kde.org>> wrote:
> 
>     On 05/18/2010 12:30 PM, Vishesh Handa wrote:
>     >     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?
> 
>     ok, will do.
> 
>     >     [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?
> 
>     I thought that conceptually it is the resourcemanager maintaining the
>     resourcedata objects. So it should also be the one to decide when one
>     has to be deleted.
> 
> 
> Okay. I've changed it.
> 
> The shouldBeDeleted function is currently called twice. Two times in
> Resource (destructor and operator==), and once in ResourceData::replaceWith.
> 
> - Vishesh Handa
> 
> 
>     >     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 embedded and charset-unspecified text was scrubbed...
Name: 1.diff
Url: http://mail.kde.org/pipermail/nepomuk/attachments/20100519/e1178bb4/attachment-0001.bat