Port KAnimatedSystemTray into KSystemTray

[Friedrich W. H. Kossebau]

Hi,

(oh dear, my second KDE related email this week, and again it is about code 
style. I must be obsessed this week. Perhaps I have seen too much of 
improvable code.)

Am Samstag, 13. September 2008, um 18:15 Uhr, schrieb Aaron J. Seigo:
> rationale: we have a style guide for a reason =) mostly just be sure to use
> braces around even single liners, e.g.:

Aha! So this is the reason, the brace-and-hug-all-and-me party just wanna rule 
the world.

Please, don't hide the fact that that guide is _not_ an official guide. 
Inofficial especially for such controversable things like bracing single 
lines (me being a member of the 
no-braces-for-single-lines-and-braces-on-own-line party).

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

(Hm. Really, how is this better? But this is OT these days).

Isn't the problem here rather the if-return-else-return, which makes the code 
improvable?

May I make some propaganda for another style? Thanks:

Calculate the condition and store it in a const variable with a descriptive 
name. Then use this variable for the return value. 
Should be much more readable and selfdocumenting, IMHO:

bool KSystemTrayIcon::isPlaying() const
{
	if (!d->movie)
		return false;

	const bool movieIsRunning = ( d->movie->state() == QMovie::Running );
	return movieIsRunning;
}

Here it would be even possible to include the invalid state check at the start 
of the method, e.g. by this:

bool KSystemTrayIcon::isPlaying() const
{
	const bool hasMovieAndIsRunning =
		( d->movie != 0
	 	  && (d->movie->state() == QMovie::Running) );

	return hasMovieAndIsRunning;
}

The deeper logic behind this is that you separate concerns and make them 
reducable.

In your code you mix the concern of returning a value and the concern of 
calculating the state. By separating them and reducing it to a simple 
interface things are easier to capable by humans.

Concern of state: just calculating, then reduced to the const variable, needs 
no more thinking how value of variable is reached, it's just there, free your 
mind for the next thing:
Concern of returning a value: return the state variable.
Simple. No brainer. Easier to modify.

Much more important for good code than where to put braces, no?

Cheers
Friedrich
-- 
Okteta - KDE Hex Editor, coming to you with KDE 4.1