KInstance redesign

[David Faure]

On Friday 19 January 2007 13:15, Matthias Kretz wrote:
> First I think it's hard to understand what KInstance actually is supposed to 
> do. Perhaps the first step would be to define what it's used for and then 
> give it a better name. Here's how I understand it:
> - without components KInstance is not needed; it could be replaced by KGlobal
> - but components want some pseudo-global data, different from the host 
> application
> - also in an application there is always only one active component which can 
> be identified using KGlobal::activeInstance
Yes 
[this was added to be able to report bugs and display credits etc. for the active part in konqueror]

> name suggestion: KComponentData (i00:13:02:0F:56:37t's basically a collection 
> of KStandardDirs, KConfig and KAboutData objects that should be different per 
> component). KGlobal then would provide the methods mainComponent and 
> activeComponent instead of instance and activeInstance.
> 
> Suggestions for the API:
> - The objects should be passed by value, so that it can easily keep a refcount 
> (using QAtomic) of the internal data.
OK.

> - The ctor that takes (const QByteArray &instanceName) could look at the 
> current objects whether one with that name exists and return that one (that 
> means you can identify a component using either the instanceName or the 
> KInstance/KComponentData object).
I don't think we need that, do we? The kde3 shows that we can always pass it around
(and the refcounting will definitely help, see below), so why would we need to use
fragile lookup-by-name code? Fragile because you could pass any string even if doesn't
mean anything, which sort of lacks type-safety in a way (a KComponentData is component data,
"foobar" isn't necessarily an existing component name). Do you have an example in mind
where two pieces of code would both know the component name and yet not have access
to the componentdata directly?

> Do you think this is a good idea? Shall I go ahead and write a first patch and 
> if you like it create a work branch?

I'm always reluctant to big changes, but after some thinking I admit this one sounds right :)

The name will always sound a bit strange; with KComponentData it sounds strange when
being used in a simple application with no components, but of course in such a case we don't
see much of it anyway.

The refcounting is not only good for the staticdeleter case, but also to fix some memleaks that we currently have.
Check what I have locally:

--- xmlgui/kxmlguiclient.cpp    (revision 624871)
+++ xmlgui/kxmlguiclient.cpp    (working copy)
@@ -154,6 +154,9 @@ void KXMLGUIClient::reloadXML()
         setXMLFile( file );
 }

+// ####### Shouldn't the KXMLGUIClient take ownership of the instance?
+// The code often looks like setInstance(new KInstance("foo"))
+// But of course there's also code that deletes the instance itself...
 void KXMLGUIClient::setInstance( KInstance *instance )
 {
   d->m_instance = instance;

All fixed by refcounting instead, which makes sense given that the instance^H^Hcomponent data
is used (and stored) by many different classes.

> - KApplication does not inherit KInstance anymore but provides convenience 
> functions that forward the calls to the mainComponent object
Those should be marked as deprecated, too, or shouldn't be even there.
kapp->dirs() should really be ported to KGlobal::dirs(), so that it doesn't crash if kapp is 0
(widgets in designer, unit tests, etc.)

KApplication should of course still _create_ a KComponentData (accessible later via KGlobal::mainComponent())
so that in most apps main() doesn't change from what it is now.

-- 
David Faure, faure at kde.org, sponsored by Trolltech to work on KDE,
Konqueror (http://www.konqueror.org), and KOffice (http://www.koffice.org).