File dialog layout

[Aaron J. Seigo]

On Saturday 13 September 2008, Adrien BUSTANY wrote:
> But then what happens for people who want to compile things in the
> playground which use AnnotationPlugins ?

... they should get the code from kdelibs?

> Just some detail about the infrastructure of the annotation plugins :
> The "final user" (ie the developper using the plugins) uses a factory
> classes, which will return plugins. Plugins all share a common
> interface, but they don't install any header, you cannot instantiate
> them directly. So you can't just move the File AnnotationPlugin to the
> dialog tree, you need the whole code (and even if you could, the
> resource inspector needs it).

yes, the consumer needs the factory class header, the plugin needs the plugin 
class header ... it would probably be safer to just require people to have a 
copy of those headers around somewhere (e.g. "manually" installed; it's two 
files, not hard ;)

looking at the classes, the factory class looks quite straightforward and like 
it could be ready for kdelibs in very little time. the plugin class looks like 
it could use some api review and has greater possibility for BIC changes as it 
has quite a few virtuals.

looking at the plugin class, some things that came to mind:

* isReady / name / iconName are all virtual. this means that if you ever want 
to add some shared logic to isReady, for instance, you can't. it also means 
every plugin needs to have at least three lines in their header and 6 lines in 
the implementation. if you provided protected setters and made the getters 
non-virtual, not only can you add shared logic easily later on (and reduce the 
size of the virtual table =), it becomes just three lines of code in the 
plugins instead of nine (setIsReady, setName and setIconName)

* the get methods are all slots but have return values. that's a bit odd =) 
while you can certainly use slots in that way, generally slots are "call and 
forget". what's the use case for having them as slots with return values in 
this class?

* instead of getResources, etc ... we tend to drop the 'get' part in kde 
libraries.. looking at other nepomuk code, similar methods use the 'find' prefix 
(findPropertyBy*, findClassBy*). does it make sense to make that consistent 
here?

* you could combine the three getResources into just two by cobining the first 
two and providing null defaults for the last two parameters?

virtual AnnotationResult* getResources(const QString& filter, const 
Types::Property& property = Types::Property(), const Soprano::Node& object = 
Soprano::Node());

of course, that would require isEmpty or isNull checks available for 
Types::Property and Soprano::Node ... i see that at least Entity has an 
isValid method ...

* you have data members in the protected section, in particular 
m_queuedCommands. that should be accessed by a getter and a dptr should be 
added to the class. 

* the "for internal use only" enum and typdef ... would it make sense to add 
that to the private class as well, to remove it from the public API?

so nothing big there either, at least based on what's already existing... what 
were the plans for inclusion in nepomuk-kde of these classes? (timelines, etc) 
if the goal is 4.2 (*cough* *wink* *cough* ;) that could work out rather well 
for the file dialog.

p.s. does nepomuk-kde have a set coding style? the code in kdelibs isn't quite 
the kdelibs style, and annotation plugin code is yet another style .. 

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080913/f08375f9/attachment.sig>