[Qemu-devel] CODING_STYLE and if blocks

[Qemu-devel] CODING_STYLE and if blocks

[Aurelien Jarno]

Hi all,

I am currently reviewing the S390 patches which extensively use of
code like:

    if (a == 5) printf("a was 5.\n");
    else if (a == 6) printf("a was 6.\n");
    else printf("a was something else entirely.\n");

It is something currently allowed by the CODING_STYLE document (there is
no "indented statement"), but I am not fully comfortable with it. Should
we accept such code? Should we fix CODING_STYLE?

Cheers,
Aurelien

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] CODING_STYLE and if blocks

[Anthony Liguori]

Aurelien Jarno wrote:
> Hi all,
>
> I am currently reviewing the S390 patches which extensively use of
> code like:
>
>     if (a == 5) printf("a was 5.\n");
>     else if (a == 6) printf("a was 6.\n");
>     else printf("a was something else entirely.\n");
>
> It is something currently allowed by the CODING_STYLE document (there is
> no "indented statement"), but I am not fully comfortable with it. Should
> we accept such code? Should we fix CODING_STYLE?
>   

I'd vote for fixing CODING_STYLE as that syntax makes my eyes hurt.

While CODING_STYLE is there as a guideline, good taste should still 
always prevail :-)

Regards,

Anthony Liguori

> Cheers,
> Aurelien
>
>   

[Qemu-devel] Re: CODING_STYLE and if blocks

[Måns Rullgård]

Aurelien Jarno <aurelien@aurel32.net> writes:

> Hi all,
>
> I am currently reviewing the S390 patches which extensively use of
> code like:
>
>     if (a == 5) printf("a was 5.\n");
>     else if (a == 6) printf("a was 6.\n");
>     else printf("a was something else entirely.\n");
>
> It is something currently allowed by the CODING_STYLE document (there is
> no "indented statement"), but I am not fully comfortable with it. Should
> we accept such code? Should we fix CODING_STYLE?

In my opinion a line break and indent after the if () should be used
except in the rare cases where keeping it all on a single line
improves readability.

-- 
Måns Rullgård
mans@mansr.com

Re: [Qemu-devel] CODING_STYLE and if blocks

[Natalia Portillo]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

   if (a == 5)
   {
       printf("a was 5.\n");
   }
   else if (a == 6)
   {
       printf("a was 6.\n");
   }
   else
   {
       printf("a was something else entirely.\n");
   }

Is in my opinion LOT CLEANER to debug.


This is my personal preference (comments included):
   if (a == 5) // Check if it is 5
   {
       printf("a was 5.\n");
   }
   else // It was not 5
   {
       if (a == 6)
       {
            printf("a was 6.\n");
       }
       else // Neither was 6
       {
            printf("a was something else entirely.\n");
       }
   }

El 22/10/2009, a las 23:01, Aurelien Jarno escribió:

> Hi all,
>
> I am currently reviewing the S390 patches which extensively use of
> code like:
>
>   if (a == 5) printf("a was 5.\n");
>   else if (a == 6) printf("a was 6.\n");
>   else printf("a was something else entirely.\n");
>
> It is something currently allowed by the CODING_STYLE document  
> (there is
> no "indented statement"), but I am not fully comfortable with it.  
> Should
> we accept such code? Should we fix CODING_STYLE?
>
> Cheers,
> Aurelien
>
> -- 
> Aurelien Jarno                          GPG: 1024D/F1BCDB73
> aurelien@aurel32.net                 http://www.aurel32.net
>
>

[-- Attachment #2: Type: text/html, Size: 3109 bytes --]

Re: [Qemu-devel] CODING_STYLE and if blocks

[Alexander Graf]

On 23.10.2009, at 00:42, Anthony Liguori wrote:

> Aurelien Jarno wrote:
>> Hi all,
>>
>> I am currently reviewing the S390 patches which extensively use of
>> code like:
>>
>>    if (a == 5) printf("a was 5.\n");
>>    else if (a == 6) printf("a was 6.\n");
>>    else printf("a was something else entirely.\n");
>>
>> It is something currently allowed by the CODING_STYLE document  
>> (there is
>> no "indented statement"), but I am not fully comfortable with it.  
>> Should
>> we accept such code? Should we fix CODING_STYLE?
>>
>
> I'd vote for fixing CODING_STYLE as that syntax makes my eyes hurt.
>
> While CODING_STYLE is there as a guideline, good taste should still  
> always prevail :-)

I think Uli only wrote the code as is because CODING_STYLE told him to  
always use braces around one-liner statements. I don't see how

if (a == 5)
     printf("a was 5.\n");
else  if (a == 6)
     printf("a was 6.\n");
else
     printf("a was something else entirely.\n");

would be not readable. In fact I tend to use that code style myself a  
lot in places where it makes sense like:

if (r < 0)
     return r;

It would really hurt my eyes to have braces on these simple ifs every  
single time.

Alex

Re: [Qemu-devel] CODING_STYLE and if blocks

[Aurelien Jarno]

On Fri, Oct 23, 2009 at 01:47:04AM +0200, Alexander Graf wrote:
>
> On 23.10.2009, at 00:42, Anthony Liguori wrote:
>
>> Aurelien Jarno wrote:
>>> Hi all,
>>>
>>> I am currently reviewing the S390 patches which extensively use of
>>> code like:
>>>
>>>    if (a == 5) printf("a was 5.\n");
>>>    else if (a == 6) printf("a was 6.\n");
>>>    else printf("a was something else entirely.\n");
>>>
>>> It is something currently allowed by the CODING_STYLE document  
>>> (there is
>>> no "indented statement"), but I am not fully comfortable with it.  
>>> Should
>>> we accept such code? Should we fix CODING_STYLE?
>>>
>>
>> I'd vote for fixing CODING_STYLE as that syntax makes my eyes hurt.
>>
>> While CODING_STYLE is there as a guideline, good taste should still  
>> always prevail :-)
>
> I think Uli only wrote the code as is because CODING_STYLE told him to  
> always use braces around one-liner statements. I don't see how
>
> if (a == 5)
>     printf("a was 5.\n");
> else  if (a == 6)
>     printf("a was 6.\n");
> else
>     printf("a was something else entirely.\n");
>
> would be not readable. In fact I tend to use that code style myself a  
> lot in places where it makes sense like:
>
> if (r < 0)
>     return r;
>
> It would really hurt my eyes to have braces on these simple ifs every  
> single time.
>

It has been debated already a few times already, the argument against 
this it that it make patches more difficulty readable.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net