Style niggle round 2

[Lei Zhang]

Dear Chromium-dev,

Here is another point that the style guide is not clear on. When we have:

#if condition_foo
...
#else
...
#endif // comment_in_questions

should comment_in_question be:

1. condition_foo, so we know which #if the #endif refers to.

or

2. !condition_foo, so we know what conditions satisfy the #else block.

[Peter Kasting]

Probably #2 if you also add "// !condition_foo" after the #else.

PK

[Evan Stade]

I agree with this, although after looking through the code #1 seems
more common currently

> PK
>
> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
>

[Brett Wilson]

On Thu, Apr 14, 2011 at 4:28 PM, Lei Zhang <the...@chromium.org> wrote:

Personally, I recommend doing whatever you think looks clearest when
you write the code. We standardize a lot of things, which is nice, but
at a certain point it becomes too much overhead to worry about. Just
be consistent with surrounding code.

Brett

[Pam Greene]

I agree with this, but have to say that #2 is intrinsically more confusing to me. You have "if A", which is naturally matched by "end if A", not "end if !A". It's not #endelse...

- Pam

 

Brett

[Stuart Morgan]

Le 15 avril 2011 01:51, Pam Greene <p...@chromium.org> a écrit :
> I agree with this, but have to say that #2 is intrinsically more confusing
> to me. You have "if A", which is naturally matched by "end if A", not "end
> if !A". It's not #endelse...

Except that this sort of falls apart as soon as there is an #elif involved.

#if A
#elif B // A? B && !A ?
#endif // A (?!)

Add another #elif and things get extra ugly, and the chances of the
comments staying accurate over time go down.

Personally I've found that in practice since I don't know how many
#else or #elif cases there are, or what style the author used, these
comments aren't actually useful beyond helping me match lines in
nested #if constructions; they don't actually tell me what the logic
above the #endif is for in isolation. In any non-trivial case I would
actually find:

#if A // Foo
#elif B // Foo
#end // Foo

more useful since it doesn't pretend it will communicate information
about the logic that it doesn't really. As long as the nesting doesn't
get too bad either style is mentally mappable to this though, and if
the nesting is so confusing that it's not, the code probably has
deeper problems.

-Stuart

[William Chan (陈智昌)]

On Fri, Apr 15, 2011 at 7:58 AM, Brett Wilson <bre...@chromium.org> wrote:

FWIW, +1

[Peter Kasting]

On Fri, Apr 15, 2011 at 8:03 AM, Stuart Morgan <stuart...@chromium.org> wrote:

In any non-trivial case I would
actually find:

#if A  // Foo
#elif B  // Foo
#end  // Foo

more useful since it doesn't pretend it will communicate information
about the logic that it doesn't really.

Yeah, this is probably true.  In fact, I personally prefer not having EOL comments at all on #ifs; the comments feel like noise and easily get out-of-date or confusing.  My original answer was not the product of much reflection :P

PK

[Scott Byer]

The rule of thumb I like to follow is if the conditionals are all flat and fit within a page (~40 lines?) so that you can see them all at once, leave off the comments - it is just noise at that point. When that's not the case, I don't mind any simple style as long as I can reverse search for the #if the #else/#endif belong to, though I like Stuart's suggestion since it serves that purpose even when things get complicated.

But if you have to scroll back and forth a few times to try and follow the logic, perhaps it's time to re-factor the conditionals into something flatter anyways.

-Scott

--

[Evan Stade]

On Fri, Apr 15, 2011 at 8:03 AM, Stuart Morgan
<stuart...@chromium.org> wrote:

what is Foo in this case? If Foo == A then it's pretty confusing for
the #else case

>
> -Stuart

[Stuart Morgan]

Le 15 avril 2011 11:05, Evan Stade <est...@chromium.org> a écrit :
> what is Foo in this case? If Foo == A then it's pretty confusing for
> the #else case

Foo in my example is an arbitrary string that's not at attempt to
describe the logic, precisely so that there's no confusion as to which
sense it's being used in. It's just a marker that allows someone to
quickly find the other associated lines, particularly if there are
nested #if's.

-Stuart

[James Robinson]

That sounds like a problem the editor should solve.

- James