Re: [ros-dev] [ros-diffs] [hbelusca] 58902: [FREELDR] - Fix some function parameter types, so to avoid unuseful casts (from pointers to const strings to pointers to strings in particular). - The big thing: make FreeLdr load...

[Timo Kreuzer]

I'm not a fan of single line conditional statements.
I'd prefer if we had a style rule for that.
Any thoughts on that?


Am 01.05.2013 19:12, schrieb hbelusca(a)svn.reactos.org:
>   Author: hbelusca
>  Date: Wed May  1 17:12:56 2013
>  New Revision: 58902 
>     	do
>    	{
>  -		while (*LoadOptions == '/')
>  -			++LoadOptions;
>  -
>  +		while (*LoadOptions == '/') ++LoadOptions;
>    		*NewLoadOptions++ = *LoadOptions;
>    	} while (*LoadOptions++);
>    
>

[Zachary Gorden]

Not opposed to them, but if we allow them, perhaps more space in between
the actual conditional and the operation?


On Wed, May 1, 2013 at 1:31 PM, Timo Kreuzer <timo.kreuzer(a)web.de> wrote:

>  
>  I'm not a fan of single line conditional statements.
>  I'd prefer if we had a style rule for that.
>  Any thoughts on that?
>
>
>  Am 01.05.2013 19:12, schrieb hbelusca(a)svn.reactos.org:
>
> >   Author: hbelusca
> >  Date: Wed May  1 17:12:56 2013
> >  New Revision: 58902
> >  
>           do
> >           {
> >  -               while (*LoadOptions == '/')
> >  -                       ++LoadOptions;
> >  -
> >  +               while (*LoadOptions == '/') ++LoadOptions;
> >                  *NewLoadOptions++ = *LoadOptions;
> >          } while (*LoadOptions++);
> >
> >  
>
>  ______________________________**_________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>
> http://www.reactos.org/**mailman/listinfo/ros-dev<http://www.reactos.org…
>

[Aleksey Bragin]

I'm fine with oneliner if statemens, like
if (!NT_SUCESS(Status)) return Status;

as they actual make sense.

However, in this case with a while example, it's far from being 
readable. Also, why use preincrement instead of a postincrement, when 
the resulting value is not used?

Regards,
Aleksey Bragin

On 01.05.2013 22:31, Timo Kreuzer wrote:
>  
>  I'm not a fan of single line conditional statements.
>  I'd prefer if we had a style rule for that.
>  Any thoughts on that?
>
>
>  Am 01.05.2013 19:12, schrieb hbelusca(a)svn.reactos.org:
> >   Author: hbelusca
> >  Date: Wed May  1 17:12:56 2013
> >  New Revision: 58902 
> >         do
> >        {
> >  -        while (*LoadOptions == '/')
> >  -            ++LoadOptions;
> >  -
> >  +        while (*LoadOptions == '/') ++LoadOptions;
> >            *NewLoadOptions++ = *LoadOptions;
> >        } while (*LoadOptions++);
> >  
>

[Timo Kreuzer]

I have an actual pragmatic argument against one liners:
When you debug code (and I mean with a real debugger aka WinDbg and not 
kdbg) and you step through the source, whenever there is a one liner, 
you just don't see which branch it takes and whether it executed the 
statement or not. So you have to check other things. Look at the 
variables. And when there is stuff like
"if (FOO_MACRO(Value)) GlobalVariable++;" you are simply f***ed.
You'll have to add a watch for GlobalVariable and check the value before 
and after.
I really prefer to see what path the code takes, when stepping over it.

So it's not a question of style or beauty, but a question of convenient 
debugging.

I know it does not apply to all kinds of one liners, but I also don't 
think that one liners make the code any more readable.

Just my 2 cents.

Am 01.05.2013 22:51, schrieb Aleksey Bragin:
>   I'm fine with oneliner if statemens, like
>  if (!NT_SUCESS(Status)) return Status;
>
>  as they actual make sense.
>
>  However, in this case with a while example, it's far from being 
>  readable. Also, why use preincrement instead of a postincrement, when 
>  the resulting value is not used?
>
>  Regards,
>  Aleksey Bragin
>
>  On 01.05.2013 22:31, Timo Kreuzer wrote:
> >  
> >  I'm not a fan of single line conditional statements.
> >  I'd prefer if we had a style rule for that.
> >  Any thoughts on that?
> >
> >
> >  Am 01.05.2013 19:12, schrieb hbelusca(a)svn.reactos.org:
> > >   Author: hbelusca
> > >  Date: Wed May  1 17:12:56 2013
> > >  New Revision: 58902 
> > >         do
> > >        {
> > >  -        while (*LoadOptions == '/')
> > >  -            ++LoadOptions;
> > >  -
> > >  +        while (*LoadOptions == '/') ++LoadOptions;
> > >            *NewLoadOptions++ = *LoadOptions;
> > >        } while (*LoadOptions++);
> > >  
> >  
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>

[Hermès BÉLUSCA - MAÏTO]

No problem guys !

This got reverted in revision 58909.
Next times I will think two times before inlining such a thing.

Hermès

-----Message d'origine-----
De : ros-dev-bounces(a)reactos.org [mailto:[e-mail]] De la
part de Timo Kreuzer
Envoyé : jeudi 2 mai 2013 00:30
À : ReactOS Development List
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 58902: [FREELDR] - Fix some
function parameter types, so to avoid unuseful casts (from pointers to const
strings to pointers to strings in particular). - The big thing: make FreeLdr
load...


I have an actual pragmatic argument against one liners:
When you debug code (and I mean with a real debugger aka WinDbg and not
kdbg) and you step through the source, whenever there is a one liner, you
just don't see which branch it takes and whether it executed the statement
or not. So you have to check other things. Look at the variables. And when
there is stuff like "if (FOO_MACRO(Value)) GlobalVariable++;" you are simply
f***ed.
You'll have to add a watch for GlobalVariable and check the value before and
after.
I really prefer to see what path the code takes, when stepping over it.

So it's not a question of style or beauty, but a question of convenient
debugging.

I know it does not apply to all kinds of one liners, but I also don't think
that one liners make the code any more readable.

Just my 2 cents.

Am 01.05.2013 22:51, schrieb Aleksey Bragin:
>   I'm fine with oneliner if statemens, like if
> (!NT_SUCESS(Status)) 
>  return Status;
>
>  as they actual make sense.
>
>  However, in this case with a while example, it's far from being 
>  readable. Also, why use preincrement instead of a postincrement, when 
>  the resulting value is not used?
>
>  Regards,
>  Aleksey Bragin
>
>  On 01.05.2013 22:31, Timo Kreuzer wrote:
> >  
> >  I'm not a fan of single line conditional statements.
> >  I'd prefer if we had a style rule for that.
> >  Any thoughts on that?
> >
> >
> >  Am 01.05.2013 19:12, schrieb hbelusca(a)svn.reactos.org:
> > >   Author: hbelusca
> > >  Date: Wed May  1 17:12:56 2013
> > >  New Revision: 58902 
> > >         do
> > >        {
> > >  -        while (*LoadOptions == '/')
> > >  -            ++LoadOptions;
> > >  -
> > >  +        while (*LoadOptions == '/') ++LoadOptions;
> > >            *NewLoadOptions++ = *LoadOptions;
> > >        } while (*LoadOptions++);
> > >  
> >  
>
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>  

_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

[Sven Barth]

Am 02.05.2013 00:30, schrieb Timo Kreuzer:
>  
>  I have an actual pragmatic argument against one liners:
>  When you debug code (and I mean with a real debugger aka WinDbg and 
>  not kdbg) and you step through the source, whenever there is a one 
>  liner, you just don't see which branch it takes and whether it 
>  executed the statement or not. So you have to check other things. Look 
>  at the variables. And when there is stuff like
>  "if (FOO_MACRO(Value)) GlobalVariable++;" you are simply f***ed.
>  You'll have to add a watch for GlobalVariable and check the value 
>  before and after.
>  I really prefer to see what path the code takes, when stepping over it.
>
>  So it's not a question of style or beauty, but a question of 
>  convenient debugging. 
This is also the reason why I don't like to put a
statement in the same 
line as a if/while-condition or for-loop-header no matter what 
programming language I'm using.

Regards,
Sven

[Colin Finck]

Timo Kreuzer <timo.kreuzer(a)web.de> wrote:
>   I'm not a fan of single line conditional
> statements.
>  I'd prefer if we had a style rule for that. 
A good opportunity to dig out our almost 5-year-old coding style 
proposal. Chances are even better that we finally get it through now 
that Aleksey has recently sacrificed the 80-character line limit we were 
unsure about :-)

Here's the link: 
https://docs.google.com/document/d/1qIWw6FN2mw-Fws2lTFDUZ0RV1nvL44VccN4C2oe…
It was pretty much finished last time, but please recheck now that 
things may have changed. Give comments and invite everybody who could be 
interested. Finally, this shall be published on the Wiki or Drupal.


- Colin

[Hermès BÉLUSCA - MAÏTO]

Excellent idea !!!!!!!!!!

Since I've already saw too much code not fully compliant with the already
laxist code styling document present in our website, which allows everybody
to do almost what he wants in the code.
I will have a look at it and maybe add, if I'm allowed, comments on existing
propositions there.

Hermès

-----Message d'origine-----
De : ros-dev-bounces(a)reactos.org [mailto:[e-mail]] De la
part de Colin Finck
Envoyé : jeudi 2 mai 2013 18:21
À : ros-dev(a)reactos.org
Objet : Re: [ros-dev] [ros-diffs] [hbelusca] 58902: [FREELDR] - Fix some
function parameter types, so to avoid unuseful casts (from pointers to const
strings to pointers to strings in particular). - The big thing: make FreeLdr
load...

Timo Kreuzer <timo.kreuzer(a)web.de> wrote:
>   I'm not a fan of single line conditional
> statements.
>  I'd prefer if we had a style rule for that. 
A good opportunity to dig out our almost 5-year-old coding style proposal.
Chances are even better that we finally get it through now that Aleksey has
recently sacrificed the 80-character line limit we were unsure about :-)

Here's the link: 
https://docs.google.com/document/d/1qIWw6FN2mw-Fws2lTFDUZ0RV1nvL44VccN4C2oe5
kIU/edit?usp=sharing
It was pretty much finished last time, but please recheck now that things
may have changed. Give comments and invite everybody who could be
interested. Finally, this shall be published on the Wiki or Drupal.


- Colin

_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev