On 05/09/2010 02:37 PM, Vishesh Handa wrote: > *4.* In *ResourceData::resetAll* - shouldn't *m_cacheDirty = false *be > equal to true? Considering resetAll is called when the resource is being > Removed, it shouldn't make a any difference. Still, just to be on the > safer side.
This would not make much sense since the Resource becomes invalid and has no URI to check for any data to load. > *5. Error Checking: *There is considerable error checking in > Nepomuk::ResouceFilterModel where /Soprano::Error/s are returned, but > after that it all stops. This is a bad situation indeed. However, I am not sure how to fix that at the moment. The only solution I see would be to emit signals from ResourceManager reporting the problems - although that does not at all fit with the rest of the sync api. > Particularly in ResourceData and Resource. We > could make the Resource class return Error codes or simply boolean > values in functions like SetProperty. That is not possible due to binary compatibility. > That would mean additional error checking from the users side, and would > result in ugly code. This is one of cases where I would love to use > exceptions. I agree. Here exceptions would be nice. > *6. Destructors: *Shouldn't ResourceManagerPrivate have a destructor > where it clears out the remaining ResouceData in m_initializedData and > other? I know ResourceData::resetAll removes them, but it feels wrong > not to check them just in case. Maybe we should rather put a Q_ASSERT(m_initializedData.isEmpty()) in there. > *7.* *ResourceManagerPrivate::dataCacheFull()*: Why 1000? I think this > value should be customizable. That is a random value I chose. I would be fine with a customization. But I doubt it would ever be used. > *8.* *ResourceManager::allResourcesWithProperty*: They seem to be using > a QList internally and using the QList:contains() functions quite > frequently. Considering QList isn't sorted, i think it would be quite > costly -> O(n). How about using a QSet? The only problem is the added > QSet::toList() in the end, which shouldn't be too slow. (Max O(n)). I've > provided a patch. please apply. :) Cheers, Sebastian _______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
