Extender api review, round 2

[Rob Scheepmaker]

On Wednesday 30 July 2008 21:12:15 Aaron J. Seigo wrote:
> good point, i wonder what the use of the hovert events are now as well, now
> that you mention it.

Personally I quite like just looking at the virtuals when you want to subclass a class. Those 4 events 
are the 4 function you'll need to implement when you want to subclass extender to do for example stuff 
with gridlayouts, super funky world clock madness, stuff like that. The hover events insert spacers and 
stuff.

> > >             void addAction(const QString &name, QAction *action);
> > >             QAction *action(const QString &name) const;
> >
> > Hmmm, I was wondering what it was for, so I looked at the apidox (yeah,
> > cheater!). It asks for a rename I guess. I would propose something like:
> > void addHandleAction(const QString &name, QAction *action);
> > QAction *handleAction(const QString &name) const;
>
> i don't agree; this exposes an implementation detail (that the action is
> used in the handle) and addAction/action are consistent with QWidget API.

Makes sense... people might even implement ExtenderItems one day that don't even show their actions in 
the draghandle.

> > > class PLASMA_EXPORT Applet : public QGraphicsWidget
> > > {
> > >     public:
> > >         virtual void initExtenderItem(ExtenderItem *item);
> >
> > I can't help but wonder if that could be in another class. Could it be
> > made a signal in Extender itself?
>
> good idea if it works; does the consumer always have access to the Extender
> object in this case? that would be my only concern. i suppose it would have
> to, since it's the creation of an Extender object in the applet that enabed
> Extender support in the applet in the first place.

I think that most of the time the function only needs the ExtenderItem.

> a signal is also a bit more flexible, as it makes it easier to connect this
> operation to other classes.

True... currently however, the extenders loads it's extender items in it's constructor. When we go for 
the signal approach, we need to connect the signal before loading the extender items, so I'lI probably 
need to move loadExtenderItems from pimpl to public api in that case. And besides: I just love 
virtuals... they feel more natural for this kind of stuff.
But there are certainly things to say for the signal approach. Are there more people who have an opinion 
about this?

> > >         Extender *extender() const;
> >
> > I would remove this one. If someone needs to access its extender often
> > he'll keep a pointer around.
>
> and what about items outside of the Applet that would like access to the
> Extenders? i wonder if Containment, Corona or any other classes would
> benefit from that?

Hmm, I can't think of a usecase for this, but there might be.
>
> on the down side:
> 	it's one more pointer in each applet.
that's not really an issue.
> 	it enforces a one extender per applet mechanism
>
> on the up side:
> 	using an extender is simply a matter of:
> 		new Extender(this)
yeah, I love being able to do that... it's short and pleasing to the eye :)
> 	it enforces a one extender per applet mechanism ;)

Ok, all in all, I don't think we really need extender(). ExtenderItem uses that (so when we find which 
applet we're dropped on, we know which extender to add the item to), but can be friend of applet. But I 
just love being able to new Extender(this)

I would love to keep extender() but not for really rational reasons. So I'll probably end up removing 
it... Any other people who have an opinion about this?