Some other things -
*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.
*5. Error Checking: *There is considerable error checking in
Nepomuk::ResouceFilterModel where *Soprano::Error*s are returned, but after
that it all stops. Particularly in ResourceData and Resource. We could make
the Resource class return Error codes or simply boolean values in functions
like SetProperty.
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.
*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.
*7.* *ResourceManagerPrivate::dataCacheFull()*: Why 1000? I think this value
should be customizable.
*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.
- Vishesh Handa
Index: resourcemanager.cpp
===================================================================
--- resourcemanager.cpp (revision 1123963)
+++ resourcemanager.cpp (working copy)
@@ -359,7 +359,7 @@ QList<Nepomuk::Resource> Nepomuk::Resour
QList<Nepomuk::Resource> Nepomuk::ResourceManager::allResourcesOfType( const QUrl& type )
{
- QList<Resource> l;
+ QSet<Resource> set;
if( !type.isEmpty() ) {
// check local data
@@ -367,8 +367,7 @@ QList<Nepomuk::Resource> Nepomuk::Resour
for( QList<ResourceData*>::iterator rdIt = localData.begin();
rdIt != localData.end(); ++rdIt ) {
Resource res( *rdIt );
- if( !l.contains( res ) )
- l.append( res );
+ set.insert(res);
}
// kDebug() << " added local resources: " << l.count();
@@ -379,14 +378,13 @@ QList<Nepomuk::Resource> Nepomuk::Resour
while( it.next() ) {
Statement s = *it;
Resource res( s.subject().uri() );
- if( !l.contains( res ) )
- l.append( res );
+ set.insert(res);
}
// kDebug() << " added remote resources: " << l.count();
}
- return l;
+ return set.toList();
}
@@ -416,7 +414,7 @@ QList<Nepomuk::Resource> Nepomuk::Resour
QList<Nepomuk::Resource> Nepomuk::ResourceManager::allResourcesWithProperty( const QUrl& uri, const Variant& v )
{
- QList<Resource> l;
+ QSet<Resource> set;
if( v.isList() ) {
kDebug() << "(ResourceManager::allResourcesWithProperty) list values not supported.";
@@ -426,7 +424,7 @@ QList<Nepomuk::Resource> Nepomuk::Resour
QList<ResourceData*> localData = d->allResourceDataWithProperty( uri, v );
for( QList<ResourceData*>::iterator rdIt = localData.begin();
rdIt != localData.end(); ++rdIt ) {
- l.append( Resource( *rdIt ) );
+ set.insert( Resource( *rdIt ) );
}
// check remote data
@@ -444,12 +442,11 @@ QList<Nepomuk::Resource> Nepomuk::Resour
while( it.next() ) {
Statement s = *it;
Resource res( s.subject().uri() );
- if( !l.contains( res ) )
- l.append( res );
+ set.insert( res );
}
}
- return l;
+ return set.toList();
}
_______________________________________________
Nepomuk mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/nepomuk
