Port KAnimatedSystemTray into KSystemTray

[Aaron J. Seigo]

On Saturday 13 September 2008, Lukas Appelhans wrote:
> Oh sorry wrong patch

=)

suggestion: remove isPlaying()

rationale: isPlaying() can be retrieved from the movie object, and only makes 
sense for icons with movies (so it's not a general purpose method) ... perhaps 
that should be something the application using it should handle then. it's a 
one liner, so isn't a big deal: if (icon->movie() && icon->movie()->state() == 
QMove::Running)


suggestion: use setMovie in the constructor rather than pass the movie into 
the private class

rationale: setMovie connects the movie's frameChanged signal, sets the cache 
mode, etc. this doesn't happen when passing the movie into the construtor! so 
either this code would need to be duplicated (not good =) or else just use 
setMovie in the KSystemTrayIcon constructor that takes a QMovie*


suggestion: reparent the movie to the tray icon

rationale: instead of deleting the movie member directly, reparent it in 
setMovie: movie->setParent(this); not only does this make the memory 
management a bit easier (less code), this will prevent crashes where people do 
something like:

new KSystemTrayIcon(new QMovie(pathToMovie, someObject), this);

and then later delete someObject, causing the movie to be deleted as well 
which would then lead to a crash in KSystemTrayIcon



suggestion: make slotNewFrame a Q_PRIVATE_SLOT, move to private class

rationale: avoid poluting the public API with private symbols



suggestion: (this is a minor one =) use the kdelibs coding style

rationale: we have a style guide for a reason =) mostly just be sure to use 
braces around even single liners, e.g.:

+    if (d->movie->state() == QMovie::Running)
+        return true;
+    else
+        return false;


should be:

+    if (d->movie->state() == QMovie::Running) {
+        return true;
+    } else {
+        return false;
+    }

there are some other bits of whitespace here and there, but i don't want to 
get too picky here ;)

-- 
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: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20080913/7d91ac25/attachment.sig>