Port KAnimatedSystemTray into KSystemTray

Friedrich W. H. Kossebau kossebau at kde.org
Sat Sep 13 21:01:42 BST 2008


(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 

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 

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?

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

More information about the kde-core-devel mailing list