New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conditionals Assertion: Citation Needed #26

Closed
jnozzi opened this Issue Aug 4, 2013 · 11 comments

Comments

Projects
None yet
7 participants
@jnozzi

jnozzi commented Aug 4, 2013

"Conditional bodies should always use braces even when a conditional body could be written without braces (i.e., it is one line only) braces should still be used. There are just too many little ways to get burned otherwise."

Please elaborate with at least one way to get burned that doesn't exist independently of bracing one-line conditional bodies. Otherwise, this is merely dogma and should at least be labeled "because we say so". Example: "...because then you might accidentally add a line after it that you expect to be part of the conditional." To which I'd say, "bullshit - the editor highlight this with its auto-indentation (because all code-aware editors I've used 'correctly' indent one-liners and not subsequent lines) and therefore this "problem" was solved long ago by a much higher and much-more-deeply-entrenched standard.

While I've never been a fan of the truly one-line if (someCondition) someExpression(); style because I prefer seeing an indentation to set off a body visually, I see nothing wrong with one-line bodies without braces after the test. In fact, it's more succinct and takes much less effort when scanning code with the eyes than all the visual weight of a braced body (the braces, of course, being there specifically because a way to group is needed).

So: my issue is mainly with the unbacked assertion that unbraced conditional bodies are somehow more dangerous than adding extraneous characters for a body that may never grow beyond that single line. It seems more like premature optimization and over-engineering to me.

@ghost ghost assigned bcapps Aug 4, 2013

@robrix

This comment has been minimized.

Show comment
Hide comment
@robrix

robrix Aug 4, 2013

Contributor

It can also be a problem if you use multi-statement macros that are written poorly. For example:

#define foo(x, y) x; y

if (bar) foo(quux(), quux());

This is, of course, a better argument for writing multi-statement macros correctly:

#define foo(x, y) do { x; y; } while(0)

(Lots of folks don’t understand why they see do/while in macro expansions, or more recently, GNU C statement expressions or blocks. This is why.)

Contributor

robrix commented Aug 4, 2013

It can also be a problem if you use multi-statement macros that are written poorly. For example:

#define foo(x, y) x; y

if (bar) foo(quux(), quux());

This is, of course, a better argument for writing multi-statement macros correctly:

#define foo(x, y) do { x; y; } while(0)

(Lots of folks don’t understand why they see do/while in macro expansions, or more recently, GNU C statement expressions or blocks. This is why.)

@robrix

This comment has been minimized.

Show comment
Hide comment
@robrix

robrix Aug 4, 2013

Contributor

I would also suggest that it’s entirely possible that you’re savvier than the average bear, Josh. I can’t recall instances where I’ve witnessed this burn a novice, or in fact any instances where I’ve seen it burn someone without the macro thing above being involved, but I could see it occurring.

Personally, I feel like thoughtful style guides are most helpful to the least experienced, but that does not necessarily equate to them being unhelpful to the masters of the craft.

Contributor

robrix commented Aug 4, 2013

I would also suggest that it’s entirely possible that you’re savvier than the average bear, Josh. I can’t recall instances where I’ve witnessed this burn a novice, or in fact any instances where I’ve seen it burn someone without the macro thing above being involved, but I could see it occurring.

Personally, I feel like thoughtful style guides are most helpful to the least experienced, but that does not necessarily equate to them being unhelpful to the masters of the craft.

@jnozzi

This comment has been minimized.

Show comment
Hide comment
@jnozzi

jnozzi Aug 4, 2013

Great point, Rob. Regarding "most helpful to the least experienced" I'd argue "only if the style guide adequately explains why this is the better way." In its present state this assertion reads more like dogma. Particularly the "just too many ways" sentiment.

jnozzi commented Aug 4, 2013

Great point, Rob. Regarding "most helpful to the least experienced" I'd argue "only if the style guide adequately explains why this is the better way." In its present state this assertion reads more like dogma. Particularly the "just too many ways" sentiment.

@robrix

This comment has been minimized.

Show comment
Hide comment
@robrix

robrix Aug 4, 2013

Contributor

Yes, thorough justification for style choices (or at least explanation, for those choices which are just “we like looking at it better when it looks like this”) is an important tool to educate about the language. Not merely rote memorization of a pattern, cargo culting the way to prosperity as it were, but rather informing. One ought to respect one’s peers enough to explain one’s reasoning, after all.

Contributor

robrix commented Aug 4, 2013

Yes, thorough justification for style choices (or at least explanation, for those choices which are just “we like looking at it better when it looks like this”) is an important tool to educate about the language. Not merely rote memorization of a pattern, cargo culting the way to prosperity as it were, but rather informing. One ought to respect one’s peers enough to explain one’s reasoning, after all.

@chrisladd

This comment has been minimized.

Show comment
Hide comment
@chrisladd

chrisladd Aug 4, 2013

Contributor

I would argue that using conditionals without braces introduces risk when refactoring.

I also feel that it is always more clear to use braces than to not use braces. It's for the same reason that I favor using parentheses even when the order of operations would do the same thing anyway.

For example,

x = (7 * 7) - 4;

is functionally equivalent to

x = 7 * 7 - 4;

but a novice programmer, or even an experienced one, might have to think a moment before realizing the latter would produce 45, not 21. We make a hard and fast rule for braces for the simple reason that it leaves little doubt which code should be executed. In addition, it sidesteps issues with moving code around during refactoring.

Contributor

chrisladd commented Aug 4, 2013

I would argue that using conditionals without braces introduces risk when refactoring.

I also feel that it is always more clear to use braces than to not use braces. It's for the same reason that I favor using parentheses even when the order of operations would do the same thing anyway.

For example,

x = (7 * 7) - 4;

is functionally equivalent to

x = 7 * 7 - 4;

but a novice programmer, or even an experienced one, might have to think a moment before realizing the latter would produce 45, not 21. We make a hard and fast rule for braces for the simple reason that it leaves little doubt which code should be executed. In addition, it sidesteps issues with moving code around during refactoring.

@chrisladd

This comment has been minimized.

Show comment
Hide comment
@chrisladd

chrisladd Aug 4, 2013

Contributor

@bcapps , look at us overlapping like that! Excellent addition.

Contributor

chrisladd commented Aug 4, 2013

@bcapps , look at us overlapping like that! Excellent addition.

@bcapps

This comment has been minimized.

Show comment
Hide comment
@bcapps

bcapps Aug 4, 2013

Contributor

@chrisladd 👍

Contributor

bcapps commented Aug 4, 2013

@chrisladd 👍

@jnozzi

This comment has been minimized.

Show comment
Hide comment
@jnozzi

jnozzi Aug 4, 2013

All very reasonable explanations. However, is this a style guide or an Objective-C programming book? :-) What I mean is why pick on this and not nearly everything else you could do wrong due to a fundamental misunderstanding of your stated profession?

It's easy enough to comment your way out of logic anywhere and sure, use of a poorly-worded macro would confuse even the pros for at least a moment, but I'm under the opinion that a style guide is just a style guide and not a golden rulebook of language-specific right and wrong. Sure the two relate but any document that combines subjective opinion so closely to deterministic procedure is asking for criticism.

I'm enjoying the perspective, though - I honestly don't think of a style guide as being responsible for teaching a programmer flow control. :-)

jnozzi commented Aug 4, 2013

All very reasonable explanations. However, is this a style guide or an Objective-C programming book? :-) What I mean is why pick on this and not nearly everything else you could do wrong due to a fundamental misunderstanding of your stated profession?

It's easy enough to comment your way out of logic anywhere and sure, use of a poorly-worded macro would confuse even the pros for at least a moment, but I'm under the opinion that a style guide is just a style guide and not a golden rulebook of language-specific right and wrong. Sure the two relate but any document that combines subjective opinion so closely to deterministic procedure is asking for criticism.

I'm enjoying the perspective, though - I honestly don't think of a style guide as being responsible for teaching a programmer flow control. :-)

@mattbischoff mattbischoff closed this in #27 Aug 4, 2013

@kballard

This comment has been minimized.

Show comment
Hide comment
@kballard

kballard Oct 17, 2014

The macro justification here is wrong. Multi-statement macros that are written like that are unequivocally incorrect, and can cause problems in other use-cases too. It is not an appropriate justification for requiring braces, it is merely a justification for requiring that macros be written correctly.

Requiring braces when a new line is involved is eminently reasonable, e.g.

if (foo)
    bar;

There are a number of possible justifications for requiring the braces. But they don't apply to the single-line variant

if (foo) bar;

There's no risk of introducing errors during refactoring or moving code, or incorrect indentation causing issues, and no risk of "goto fail".

That said, single-line if conditions should be used sparingly. They can increase code legibility, but they can also decrease it. But the only good reason to ban it is "because we don't want to make subjective code legibility calls", although the style guide already contains other rules that require code legibility calls (such as the one on the ternary operator).

kballard commented Oct 17, 2014

The macro justification here is wrong. Multi-statement macros that are written like that are unequivocally incorrect, and can cause problems in other use-cases too. It is not an appropriate justification for requiring braces, it is merely a justification for requiring that macros be written correctly.

Requiring braces when a new line is involved is eminently reasonable, e.g.

if (foo)
    bar;

There are a number of possible justifications for requiring the braces. But they don't apply to the single-line variant

if (foo) bar;

There's no risk of introducing errors during refactoring or moving code, or incorrect indentation causing issues, and no risk of "goto fail".

That said, single-line if conditions should be used sparingly. They can increase code legibility, but they can also decrease it. But the only good reason to ban it is "because we don't want to make subjective code legibility calls", although the style guide already contains other rules that require code legibility calls (such as the one on the ternary operator).

@paulbruneau

This comment has been minimized.

Show comment
Hide comment
@paulbruneau

paulbruneau Oct 17, 2014

Contributor

I disagree that there's no risk of introducing errors. I would say it's quite high relatively speaking. A single line if() often becomes a multiline one and I don't want anyone to have to remember to retroactively add braces.

Contributor

paulbruneau commented Oct 17, 2014

I disagree that there's no risk of introducing errors. I would say it's quite high relatively speaking. A single line if() often becomes a multiline one and I don't want anyone to have to remember to retroactively add braces.

@mattbischoff

This comment has been minimized.

Show comment
Hide comment
@mattbischoff

mattbischoff Oct 20, 2014

Contributor

I disagree that there's no risk of introducing errors. I would say it's quite high relatively speaking

I agree with @paulbruneau here. I think the small benefit of concision is not worth the larger cost of having more than one way to do things and introducing subjective judgements into this part of the guide.

Contributor

mattbischoff commented Oct 20, 2014

I disagree that there's no risk of introducing errors. I would say it's quite high relatively speaking

I agree with @paulbruneau here. I think the small benefit of concision is not worth the larger cost of having more than one way to do things and introducing subjective judgements into this part of the guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment