Skip to content
This repository has been archived by the owner on May 9, 2023. It is now read-only.

Conditionals Assertion: Citation Needed #26

Closed
jnozzi opened this issue Aug 4, 2013 · 11 comments · Fixed by #27
Closed

Conditionals Assertion: Citation Needed #26

jnozzi opened this issue Aug 4, 2013 · 11 comments · Fixed by #27
Assignees

Comments

@jnozzi
Copy link

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
Copy link
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
Copy link
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
Copy link
Author

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
Copy link
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
Copy link
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.

@chrisladd
Copy link
Contributor

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

@bcapps
Copy link
Contributor

bcapps commented Aug 4, 2013

@chrisladd 👍

@jnozzi
Copy link
Author

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. :-)

@lilyball
Copy link

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
Copy link
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.

@mbbischoff
Copy link
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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants