KDEREVIEW: nowplaying dataengine and applet for plasma

[Aaron J. Seigo]

On Saturday 09 February 2008, Alex Merry wrote:
> This has been the only comment (apart from the discussion of the merits and

this was posted to k-c-d while i was in an airplane (for the next 2 days, as 
it turned out, with intermittent 'net access) so i missed it. it doesn't seem 
to have been CC'd to panel-devel until just now .. =/

looking at the engine i have a few comments:

* none of the strings are i18n'd, and none of the keys are I18N_NOOP'd.

* providing a "help" source is interesting. perhaps we should make that 
standard, though really ... that's for people designing widgets, isn't it? 
it's not particularly for the actual usage of the engine. documentation is an 
unanswered question with engines, and it would be good to provide a better 
way to do this that doesn't involve having to open up khelpcenter either.

* using DataEngine for a command sink is actually rather wrong from the design 
perspective of DataEngine. i'm sure it works, but it's really not what Engine 
was designed for. there's no feedback on success, for instance. we discussed 
this exact issue in the plasma meeting Saturday and we decided to provide a 
more appropriate interface for commands as it's a common need. but DataEngine 
is designed to provide access to data, in a one-to-many relationship which 
makes these things not so fun to do.

* the "properties" source is related to the above, obviously.

on to the applet:

    dataEngine("nowplaying")->connectSource("Players", this, 5000);

should the players source update itself? polling is Bad(tm). what should 
probably be happening in the engine is in addPlayer, the Player source should 
be being updated. that is constant data that is always available and pretty 
much required, after all. then you can get rid of the 5s poll above and just 
connect to the source normally.

    connect(this, SIGNAL(geometryChanged()), this, SLOT(sizeChanged()));

what you probably really want to do is respond to the SizeConstraint in 
constriantsUpdated. you also don't want to call updateGeometry() 
indiscriminently in constraintsUpdated(). in fact, updateGeometry is called 
for you when needed before constraintsUpdated() is called; the only time the 
applet should need to call it is when it changes its own geometry.

-- 
Aaron J. Seigo
humru othro a kohnu se
GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA  EE75 D6B7 2EB1 A7F1 DB43

KDE core developer sponsored by Trolltech
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080209/645060f4/attachment.sig>