On Mon, Nov 12, 2001 at 04:14:55PM -0500, Ian Reinhart Geiser wrote:
[...]

A few comments/questions about the code that I've see in the current CVS.

a) Why is ScriptLoader limited to KMainWindow as parent type?
   (counter example: that API makes it impossible to use it in
   kword or any other koffice component)

b) All the scripts currently seem to leak. Maybe setting the
   autoDelete property of 'm_scripts' would be a good idea
   (or much simpler: re-use QObject's parent-deletes-children-mechanism)

c) Wouldn't it make sense to put the 'ScriptLoader' class into the
   KDE namespace by K-prefixing' it?

d) Why does ScriptInterface have a setParent method? Why not re-use
   what you already have: QObject. Allocate the component
   implementing the ScriptInterface with the desired parent object
   as..erhm..parent object, as done through the factory?
   (another idea, in combination with b) , would be to allocate the
   components implementing the KScriptInterface with the script
   loader as parent object and to provide a pair of 'setScriptParent' /
   'scriptParent' -- maybe even the naming 'parent' is misleading
   here)

e) Minor API suggestion: What about renaming 'bool status()' to
   'bool isRunning()' ? (or to change status() not to return a bool
   but an enum providing more detailed information about the status of
   the current script)

f) Why does KScriptInterface, as interface, inherit from QObject at
   all?

g) Why does getScripts() (which lacks a const specifier btw :) )
   return an action object? How about 'KAction *scriptAction()
   const' along with an accessor to retrieve a list of QObjects that
   implement the KScriptInterface?
   (just an idea :)


Simon