Extender api review, round 2

[Alex Merry]

On Wednesday 30 July 2008 22:23:58 Rob Scheepmaker wrote:
> On Wednesday 30 July 2008 21:32:24 Alex Merry wrote:
> > Question: who owns Extenders?  Applet, or the creator of the Extender? 
> > If the Applet owns it, then I can't create it on the heap (ie: as a
> > non-pointer member of my subclass), and a factory method might be safer.
> > If the creator owns it, they have to keep a reference to it to clean it
> > up anyway.
>
> Applet is the creator of the extender, so I don't really get your question.

Oh, I suppose they're all QObjects, so its just the QObject parent mechanism.  
So there's no need to worry about owning or deleting.

If I wanted to have a widget as a detachable item, what would I do?  From what 
I can see of the API, it would be something like, in initExtenderItem(), I'd 
have
    if (item->name() == "MyExtender") {
        item->setWidget(new MyWidget);
    }

And in init(), I'd have something like
    Extender* extender = new Extender(this);
    ExtenderItem* item = new ExtenderItem(extender);
    item->setName("MyExtender");
    initExtenderItem(item);

Is this right?  This doesn't feel natural to me: it looks like Extender is 
just another widget/other child of the applet.  When it's really more of a 
one-off thing, like a layout.  So I'd find something like
    setExtender(new Extender);
    ExtenderItem* item = new ExtenderItem(extender());
            // or ExtenderItem(this)
    // yadda yadda
or
    setExtender(new Extender);
    ExtenderItem* item = new ExtenderItem(name);
    initExtenderItem(item);
    extender()->addItem(item);
more natural.

I also don't like the initExtenderItem() method, but I also can't see how to 
get around it without forcing people to subclass ExtenderItem or create all 
the ExtenderItems it might need in init().


>
> > Other notes:
> >
> > I don't like the term "source applet".  I think "host applet" would be
> > better.
>
> well... host applet is something different from source applet. The source
> applet is the applet that created the extender item, and is needed so
> extender can call the initExtenderItem for an extender item after a plasma
> restart. Host applet I consider to be the applet that... well... hosts the
> item. It's the applet that contains the extender that is parent the item.
> So basically: isDetached = (sourceApplet != hostApplet)

Ah, OK, that confused me.  I didn't realise what the purpose of 
sourceAppletId() was.  In which case, does it need to be public?  Only 
Extender (and possibly subclasses of ExtenderItem?) would care, surely?

Anyway, the api documentation should say something like "the id of the applet 
this item was created by", rather than "the id of the applet this item is 
contained in".

I'm afraid I'm coming at this without a clear idea of the concepts around 
extenders, except that they allow users to pull parts of applets away from the 
main widget.  Although that's possibly a good thing for API reviewing, since 
most applet creators will use the api and apidocs to try to understand what 
they're about.

Alex


-- 
KDE: http://www.kde.org
Ubuntu/Kubuntu: http://www.ubuntu.org http://www.kubuntu.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20080730/24c94cbc/attachment.pgp