[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Extender api review.
From:       Rob Scheepmaker <r.scheepmaker () student ! utwente ! nl>
Date:       2008-07-25 16:06:04
Message-ID: 200807251806.04482.r.scheepmaker () student ! utwente ! nl
[Download RAW message or body]

On Thursday 24 July 2008 00:59:01 Richard Moore wrote:
> Just the minor stuff:

Thanks for the feedback!

> Applet
> =====
>
>  bool supportDetachables() const;
> should be:
>  bool supportsDetachables() const;

agreed :)

>  void setForcePanelCollapse(bool collapse);
> This name is pretty much meaningless, it should be renamed, I'm not
> sure what is better though.

hmm, what about setAlwaysCollapseInPanel?

> bool forcePanelCollapse() const;
> For boolean flags we have a convention that the getter will be named
> something hasXX or isXX, this breaks that convention.

has/is AlwaysCollapseInPanel wouldn't really work unfortunately. isAlwaysCollapsedInPanel maybe, and 
setAlwaysCollapsedInPanel maybe? I'm not really good in thinking up good function names...

> QString noDetachablesLabel() const;
> Can someone figure out what this does from the name? I'd say no.

Aseigo had the nice suggestion of making this (set)emptyExtenderText... I think that might be clearer.

>  void setSupportDetachables(bool supportDetachables);
> should be
>  void setSupportsDetachables(bool supportDetachables);

yes indeed.

>
> DetachableWidget
> =============
>
> name() -> const
>
> bool detachable();
> should be
> bool isDetachable() const;
>
> bool collapsed() const;
> should be
> bool isCollapsed() const;

fair enough..

>
> Corona
> =====
>
>  void addExtender(Applet *extender);
> no corresponding remove?

ehm, oops...

> I suspect there are several missing slots, in the above too. In
> addition defining some Q_PROPERTIES would be a good idea. This is just
> a quick run through without paying attention to the semantics of the
> operations themselves.

agreed about the properties. I would like to hear some of your suggestions for slot though.

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
[prev in list] [next in list] [prev in thread] [next in thread]