File dialog layout

[Adrien BUSTANY]

Aaron J. Seigo a écrit :
> 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 ;)
>   
Yep, a bit more than two (you need AnnotationProperty and 
AnnotationResult and AnnotationResource, but it's not huge). I wonder if 
we could automate that installation as a "pre-install" step before 
installing the kde-nepomuk playground...
> 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)
>   
Hmm I agree on that... Definitely better than the current scheme.
> * 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?
>   
Errr that doesn't make sense anymore (those functions used to be void, a 
long time ago), I just forgot to change them to regular functions.
> * 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?
>   
considering the behaviour of the functions, findResources makes as much 
sense as getResources do (maybe even more ;) ).
> * 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. 
>   
OK too. Sebastian already made some remarks on the API, I just didn't 
have time to take them into consideration yet (I went back to school 3 
days ago).
> * 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 ..
While the API cleanup will take a bit of time, it's not such a huge 
amount of work either. I just have to block myself three hours to clean 
that up...
About the coding style : it should be the KDE coding style, except that 
parts coded by me use tabs and not spaces (I won't troll about that, and 
am OK to comply with spaces, I just have to run sed on my files).
I'm a bit in a rush now, I have some school work and a TOEFL exam to 
sort out :s I'll try to work a bit on AnnotationPlugins tonight though.

Cheers
Adrien

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080913/de25bade/attachment.htm>