Re: [Kde-pim] Coding style: block braces in switch cases

2014-01-13 Thread Sergio Martins
On Monday, January 13, 2014 17:05:04 Kevin Krammer wrote:
> 
> I've looked around and found quite a variation:
> 
> e.g. qdatetime.cpp:
> switch (...) {
> case Foo: {
> }
> }
> 
> e.g. qrasterizer.cpp:
> switch (...) {
> case Foo:
> {
> }
> }



> e.g. qcommandlineparser.cpp:
> switch (...) {
> case Foo:
> {
> }
> }
> 
> e.g. qlocale.cpp:
> switch (...) {
> case Foo: {
> }
> }

Hi,

Personally I think it doesn't matter and we shouldn't enforce such fine grained 
policy, it makes it hard to remember and I don't want to read the guidelines 
whenever I code.

In any case, if something is to be improved in the style guidelines, please 
submit it to Qt first. We took the steps to get rid of the kdepim style to get 
close to the Qt style, so lets not start diverging again.




Regards,
Sérgio Martins



Re: Coding style: block braces in switch cases

2014-01-13 Thread Kevin Funk
Am Montag, 13. Januar 2014, 17:05:04 schrieb Kevin Krammer:
> Hi all,
> 
> at KDE PIM we are cleaning up our code base from over a decade of different
> coding styles. Well, "we" means Guy Maurel, he does all the actual work :)
> 
> We follow the "kdelibs" rules which in turn are close to what Qt uses IIRC.
> 
> Now we've hit a snag. There is no common rule regarding block braces in
> cases of switch statements.
> 
> All example of switch statements show that the switch block braces start at
> the line of the switch condition, just like with if conditions or loops.
> They also show that case is aligned with switch, i.e. no intentation.
> 
> However, there seems to be no rule how to place block braces for case
> statements.
> 
> I've looked around and found quite a variation:


Interesting topic...

I always wondered about how to do it right(tm) when facing this. In this 
particular case I think ruling out the variations by method of elimination is 
the best way to go.

> e.g. qdatetime.cpp:
> switch (...) {
> case Foo: {
> }
> }

Putting another indentation level after the 'case' statement disrupts 
readability, esp. if you have other 'case' labels without curly braces. 
Consider:

switch (...) {
case Foo: {
decl;
stmt;
break;
}
case Foo2:
stmt;
break;
case Foo: {
decl;
stmt;
break;
}
}

Lots of jumping between the columns when you scan for 'break' statements, for 
example.

> e.g. qrasterizer.cpp:
> switch (...) {
> case Foo:
> {
> }
> }
>
> e.g. qcommandlineparser.cpp:
> switch (...) {
> case Foo:
> {
> }
> }

'{' on next line is not encouraged in our coding style guide. (Besides on 
function implementations, after 'class', etc.)

IMO it also just wastes vertical screen space, which I dislike (subjective, of 
course)

> e.g. qlocale.cpp:
> switch (...) {
> case Foo: {
> }
> }

My favored variation. Considering my initial example code, this would end up 
looking like:

switch (...) {
case Foo: {
decl;
stmt;
break;
}
case Foo2:
stmt;
break;
case Foo: {
decl;
stmt;
break;
}
}

The same indentation level for the '}'s is not nice, but IMO the improved 
readability of the code this indentation scheme encourages is worth it. (All 
'break' statements in one column, less "jumping" of your eye focus).

In any case, the '{' and '}' "inside" a case label is syntactic sugar anyway, 
so it should not get into your way when reading or writing code. Your compiler 
is clever enough to error out when they're missing, and it doesn't harm 
(semantically) if they're redundant.

> Any thoughts anyone?
> 
> Cheers,
> Kevin

Just my two cents.

Greets

-- 
Kevin Funk


Coding style: block braces in switch cases

2014-01-13 Thread Kevin Krammer
Hi all,

at KDE PIM we are cleaning up our code base from over a decade of different 
coding styles. Well, "we" means Guy Maurel, he does all the actual work :)

We follow the "kdelibs" rules which in turn are close to what Qt uses IIRC.

Now we've hit a snag. There is no common rule regarding block braces in cases 
of switch statements.

All example of switch statements show that the switch block braces start at 
the line of the switch condition, just like with if conditions or loops.
They also show that case is aligned with switch, i.e. no intentation.

However, there seems to be no rule how to place block braces for case 
statements.

I've looked around and found quite a variation:

e.g. qdatetime.cpp:
switch (...) {
case Foo: {
}
}

e.g. qrasterizer.cpp:
switch (...) {
case Foo:
{
}
}

e.g. qcommandlineparser.cpp:
switch (...) {
case Foo:
{
}
}

e.g. qlocale.cpp:
switch (...) {
case Foo: {
}
}

Any thoughts anyone?

Cheers,
Kevin
-- 
Kevin Krammer, KDE developer, xdg-utils developer
KDE user support, developer mentoring


signature.asc
Description: This is a digitally signed message part.