extender api review round 3

Rob Scheepmaker r.scheepmaker at student.utwente.nl
Fri Aug 1 23:57:41 CEST 2008


On Friday 01 August 2008 23:26:22 Richard Moore wrote:
> Extender
> =======
>
> setIsPopup -> setPopup

True, I actually missed that one in round 2. 
>
> should saveState() be public? Why no restoreState()?

actually... yeah, saveState could be protected. restoreState isn't needed because in itemAddedEvent you 
can read the stuff you stored in saveState and use it to add items correctly.
>
> itemHoverLeaveEvent but no itemHoverEnterEvent seems like a bad asymmetry.

true, I didn't need it before, but I'll add it right away.
>
> ExtenderItem
> ==========
>
> KConfigGroup config();
> should this be const?

ehm, no, Applets should be able to write stuff to this config group.
>
> We have:
> void setAutoExpireDelay(uint time = 0);
> bool hasAutoExpireDelay() const;
> but no getter. Seems odd.

yeah, you're right.
>
> void destroy(); Needs better API docs. How does this differ from
> simply deleting the object?

That it also remove it's config group, instead  of just deleting the object. 
>
> the protected methods have no docs.

True, I'll add them.
>
> Applet
> =====
>
> virtual void initExtenderItem(ExtenderItem *item);
>
> This api feels a bit of kludge. As documented it sounds like an API
> for dealing with abnormal failures like crashes. These should be
> avoided rather than coded for. Am I missing something?

Actually it's not for dealing with crashes, but for being able to create the extender items again after 
you've restarted plasma. Even after you reboot your computer you still want extender items that you've 
detached previously to be available.



More information about the Plasma-devel mailing list