Wallpaper class; API review requested

[Kevin Ottens]

Le Tuesday 12 August 2008, Aaron J. Seigo a écrit :
> On Monday 11 August 2008, Kevin Ottens wrote:
> > Hello,
> >
> > Same dumb method as usual from me.
> >
> > Le Tuesday 05 August 2008, Aaron J. Seigo a écrit :
> > > the affected classes are Containment, with a setter and getter for the
> > > wallpaper
> >
> > I'd probably add a setWallpaper(Wallpaper*) so that the setter/getter
> > couple becomes symetric with an extra convenience version (the one doing
> > the load for you).
>
> ownership of the Wallpaper* would then be taken by the Containment in this
> case;

Is that required?

> there are still ways to screw it up at this point. i'm always wary of
> these kinds of setters for this reason.

Yeah, indeed, I'm always pondering about silently transfering ownership. So if 
this transfer is required I might live with the non symmetric getter/setter 
pair. If it's not we'd probably have to add a parent argument in the load 
method.

> > > and the new class Wallpaper.
> >
> > A few comments:
> >  * I think it's worth it to have the "icon" property too
>
> +1
>
> >  * I don't get what the "mode" thing is about. Any more specific use
> > case? I admit I had to look for the apidox for this one. I found a small
> > blurb in the class apidox about modes, but from the relevant methods
> > there's nothing about what the modes are. Last, the class apidox didn't
> > make me figure out what they are exactly (switch to using an "Action"
> > term at some point).
>
> yes, this needs to be made clearer. the concept is that one plugin may
> provide multiple wallpaper methods.
>
> our current wallpaper renderer does Single Image, Slideshow and Plain
> Color. so, three different modes.

Gotcha. What about RenderingMode? Just "mode" doesn't convey enough 
information to me.

> >  * Depending on what we figure out regarding what the modes are, it might
> > be worth it to have a small "WallpaperMode" class (it holds already three
> > properties, and could grow).
>
> which 3 properties?

/me looks at his hand again.
OK, I can't count it seems, the 2 properties so that obviously less of a 
problem *for now*. Could that grow? I've a hard time figuring out if it's 
likely or not... But somehow I clearly imagine that modes could have more 
information attached to them, for instance "is it animated?", for decision 
taking, for instance should I fall back to a simpler mode to save power or 
bandwidth.

> >  * I don't get the action parameter from the init() method (but since I
> > didn't understand th mode thing, that's probably related).
>
> that's actually supposed to be mode, not action.

OK, thanks.

Regards.
-- 
Kévin 'ervin' Ottens, http://ervin.ipsquad.net
"Ni le maître sans disciple, Ni le disciple sans maître,
Ne font reculer l'ignorance."
-------------- 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/20080813/72273e52/attachment-0001.pgp