[Nepomuk] Nepomuk Core - Questions & Patches

[Vishesh Handa]

Hey Sebastian

I'm sorry for the late reply I was busy with college stuff.

I looked at the modified patch, and I like the fact that it works, but I
don't like the fact that we have to remember additional stuff.
DetermineUri() must not me called inside ResourceData, and that you'll
probably have to check if the ResourceData is stored() and its uri has been
determined, before calling a method. These small things then to add up, and
clutter the Resource Code.

Ideally I would have liked ResourceData to do all this checking itself, so
that when altering the Resource class we don't have to worry about it. ( But
then we already had to worry about store() ) I don't have a solution, but
I'm trying to figure out one.

I never even thought about multi-threading before this, and, yes, I too
think it will crash. ( Time to add more tests? Can we add multi-threading
tests? )  I was thinking of some way of checking if the determineUri lock
exists, and then accordingly calling determineUri() from Resource, but I
think your method of mutex locking the Resource should cover it.

I'll try to implement it (It pretty simple, right?)

- Vishesh Handa



On Thu, May 27, 2010 at 5:29 PM, Sebastian Trüg <trueg at kde.org> wrote:

> Hi Vishesh,
>
> I finally looked at the remove-proxy patch again and fixed it. Now the
> only problem left is the design weirdness of ResourceData instances
> deleting themselves.
>
> The main thing I did was removing all calls to determineUri from
> ResourceData and moving them into Resource. I did the same with load()
> and store() which is actually not necessary. Maybe it would be cleaner
> to move those back...
>
> I did not look into the merging of load and determineUri yet. I think we
> should do that in a separate patch only after we cleaned this one up.
> Otherwise it is impossible to debug.
>
> Cheers,
> Sebastian
>
> On 05/26/2010 12:52 PM, Vishesh Handa wrote:
> >
> >
> > On Wed, May 26, 2010 at 3:13 PM, Sebastian Trüg <trueg at kde.org
> > <mailto:trueg at kde.org>> wrote:
> >
> >     On 05/26/2010 10:08 AM, Vishesh Handa wrote:
> >     >     > 3. When checking if the the m_kickoffUri is a nie:url for a
> >     resource.
> >     >     > The nieUrl is assigned to be equal to the uri. This however
> >     gets fixed
> >     >     > in the load(). And nieUrl isn't used anywhere, so it doesn't
> >     >     matter that
> >     >     > much.
> >     >
> >     >     I suppose you mean this part:
> >     >
> >     >     if( it.next() ) {
> >     >          QUrl uri = it["r"].uri();
> >     >          if( uri.isEmpty() ) {
> >     >             m_uri = m_kickoffUri;
> >     >          }
> >     >          else {
> >     >             m_uri = uri;
> >     >             m_nieUrl = uri;
> >     >          }
> >     >
> >     >     The last two lines. AFAICT this is perfectly fine since in the
> >     latter
> >     >     case both nie:url and resource URI are equal.
> >     >
> >     >
> >     > I respectfully disagree. :)
> >     >
> >     > The query used is this "select distinct ?r ?o where { { ?r nie:url
> >     <uri>
> >     > . } UNION { <uri> ?p ?o . }  } LIMIT 1". The case where ?r isn't
> empty
> >     > is when the <uri> contains the nie:url and therefore ?r will
> >     contain the
> >     > resource uri.
> >
> >     You are of course correct. It should be "m_nieUrl = m_kickoffUri"
> >     instead. Agreed?
> >
> >
> > Yup. :)
> >
> > If you get the time could you please look at my horrible merge patch?
> >
> > Thanks
> > - Vishesh Handa
> >
> >     Cheers,
> >     Sebastian
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/nepomuk/attachments/20100529/f289720a/attachment.htm