Skip to content

Commit 41091c6

Browse files
DavisVaughanlionel-hadley
authored
Adjust rules for control flow (#240)
* Adjust rule for single line vs multiline if statements * Respond to feedback, and pull out Braced Expressions into its own section * Move `Long function calls` under `Function calls` * Move implicit type coercion into if statement section * Expand semicolon section a bit * Update syntax.qmd Co-authored-by: Lionel Henry <lionel.hry@gmail.com> * Apply suggestions from code review Co-authored-by: Hadley Wickham <h.wickham@gmail.com> * Apply code review suggestions --------- Co-authored-by: Lionel Henry <lionel.hry@gmail.com> Co-authored-by: Hadley Wickham <h.wickham@gmail.com>
1 parent 315943b commit 41091c6

File tree

2 files changed

+177
-113
lines changed

2 files changed

+177
-113
lines changed

functions.qmd

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ add_two <- function(x, y) {
154154
}
155155
```
156156

157-
Return statements should always be on their own line because they have important effects on the control flow. See also [inline statements](#inline-statements).
157+
Return statements should always be on their own line because they have important effects on the control flow. See also [control flow modifiers](#control-flow-modifiers).
158158

159159
```{r}
160160
# Good

syntax.qmd

Lines changed: 176 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -292,24 +292,85 @@ The only exception is in functions that capture side-effects:
292292
output <- capture.output(x <- f())
293293
```
294294

295+
### Long function calls
295296

296-
## Control flow
297+
Strive to limit your code to 80 characters per line. This fits comfortably on a
298+
printed page with a reasonably sized font. If you find yourself running out of
299+
room, this is a good indication that you should encapsulate some of the work in
300+
a separate function or use early returns to reduce the nesting in your code.
301+
302+
If a function call is too long to fit on a single line, use one line each for
303+
the function name, each argument, and the closing `)`.
304+
This makes the code easier to read and to change later.
305+
306+
```{r}
307+
# Good
308+
do_something_very_complicated(
309+
something = "that",
310+
requires = many,
311+
arguments = "some of which may be long"
312+
)
313+
314+
# Bad
315+
do_something_very_complicated("that", requires, many, arguments,
316+
"some of which may be long"
317+
)
318+
```
319+
320+
As described under [Named arguments](#argument-names), you can omit the argument names
321+
for very common arguments (i.e. for arguments that are used in almost every
322+
invocation of the function). If this introduces a large disparity between the line lengths, you may want to supply names anyway:
323+
324+
```{r}
325+
# Good
326+
my_function(
327+
x,
328+
long_argument_name,
329+
extra_argument_a = 10,
330+
extra_argument_b = c(1, 43, 390, 210209)
331+
)
332+
333+
# Also good
334+
my_function(
335+
x = x,
336+
y = long_argument_name,
337+
extra_argument_a = 10,
338+
extra_argument_b = c(1, 43, 390, 210209)
339+
)
340+
```
341+
342+
You may place multiple unnamed arguments on the same line if they are closely
343+
related to each other. A common example of this is creating strings
344+
with `paste()`. In such cases, it's often beneficial to match one line of code
345+
to one line of output.
297346

298-
### Code blocks {#indenting}
347+
```{r}
348+
# Good
349+
paste0(
350+
"Requirement: ", requires, "\n",
351+
"Result: ", result, "\n"
352+
)
299353
300-
Curly braces, `{}`, define the most important hierarchy of R code. To make this
301-
hierarchy easy to see:
354+
# Bad
355+
paste0(
356+
"Requirement: ", requires,
357+
"\n", "Result: ",
358+
result, "\n")
359+
```
360+
361+
## Braced expressions {#braced-expressions}
362+
363+
Braced expressions, `{}`, define the most important hierarchy of R code, allowing you to group multiple R expressions together into a single expression. The most common places to use braced expressions are in function definitions, control flow, and in certain function calls (e.g. `tryCatch()` and `test_that()`).
364+
365+
To make this hierarchy easy to see:
302366

303-
* `{` should be the last character on the line. Related code (e.g., an `if`
304-
clause, a function declaration, a trailing comma, ...) must be on the same
305-
line as the opening brace.
367+
* `{` should be the last character on the line.
368+
Related code (e.g., an `if` clause, a function declaration, a trailing comma, ...) must be on the same line as the opening brace.
306369

307370
* The contents should be indented by two spaces.
308371

309372
* `}` should be the first character on the line.
310373

311-
It is occassionally useful to have an empty curly braces block, in which case it should be written `{}`.
312-
313374
```{r}
314375
# Good
315376
if (y < 0 && debug) {
@@ -340,8 +401,6 @@ tryCatch(
340401
}
341402
)
342403
343-
while (waiting_for_something()) {}
344-
345404
# Bad
346405
if (y < 0 && debug) {
347406
message("Y is negative")
@@ -355,52 +414,120 @@ if (y == 0)
355414
message("x is negative or zero")
356415
}
357416
} else { y ^ x }
417+
```
418+
419+
It is occasionally useful to have empty braced expressions, in which case it should be written `{}`, with no intervening space.
420+
421+
```{r}
422+
# Good
423+
function(...) {}
358424
359-
while (waiting_for_something()) { }
360-
while (waiting_for_something()) {
425+
# Bad
426+
function(...) { }
427+
function(...) {
361428
362429
}
363430
```
364431

432+
## Control flow
433+
434+
### Loops
435+
436+
R defines three types of looping constructs: `for`, `while`, and `repeat` loops.
437+
438+
* The body of a loop must be a braced expression.
439+
440+
```{r}
441+
# Good
442+
for (i in seq) {
443+
x[i] <- x[i] + 1
444+
}
445+
446+
while (waiting_for_something()) {
447+
cat("Still waiting...")
448+
}
449+
450+
# Bad
451+
for (i in seq) x[i] <- x[i] + 1
452+
453+
while (waiting_for_something()) cat("Still waiting...")
454+
```
455+
456+
* It is occasionally useful to use a `while` loop with an empty braced expression body to wait. As mentioned in [Braced expressions](#braced-expressions), there should be no space within the `{}`.
457+
365458
### If statements
366459

367-
* When present, `else` should be on the same line as `}`.
460+
* A single line if statement must never contain braced expressions. You can use
461+
single line if statements for very simple statements that don't have
462+
side-effects and don't modify the control flow.
368463

369-
* `&` and `|` should never be used inside of an `if` clause because they can
370-
return vectors. Always use `&&` and `||` instead.
464+
```{r}
465+
# Good
466+
message <- if (x > 10) "big" else "small"
467+
468+
# Bad
469+
message <- if (x > 10) { "big" } else { "small" }
371470
372-
* NB: `ifelse(x, a, b)` is not a drop-in replacement for `if (x) a else b`.
373-
`ifelse()` is vectorised (i.e. if `length(x) > 1`, then `a` and `b` will be
374-
recycled to match) and it is eager (i.e. both `a` and `b` will always be
375-
evaluated).
471+
if (x > 0) message <- "big" else message <- "small"
376472
377-
If you want to rewrite a simple but lengthy `if` block:
473+
if (x > 0) return(x)
474+
```
475+
476+
* A multiline if statement must contain braced expressions.
378477

379478
```{r}
479+
# Good
480+
if (x > 10) {
481+
x * 2
482+
}
483+
380484
if (x > 10) {
381-
message <- "big"
485+
x * 2
382486
} else {
383-
message <- "small"
487+
x * 3
488+
}
489+
490+
# Bad
491+
if (x > 10)
492+
x * 2
493+
494+
# In particular, this if statement will only parse when wrapped in a braced
495+
# expression or call
496+
{
497+
if (x > 10)
498+
x * 2
499+
else
500+
x * 3
384501
}
385502
```
386503

387-
Just write it all on one line:
504+
* When present, `else` should be on the same line as `}`.
505+
506+
* Avoid implicit type coercion (e.g. from numeric to logical) in the condition of an if statement:
388507

389508
```{r}
390-
message <- if (x > 10) "big" else "small"
509+
# Good
510+
if (length(x) > 0) {
511+
# do something
512+
}
513+
514+
# Bad
515+
if (length(x)) {
516+
# do something
517+
}
391518
```
392519

393-
### Inline statements {#inline-statements}
520+
::: {.callout-note}
521+
`&` and `|` should never be used inside of an `if` clause because they can return vectors. Always use `&&` and `||` instead.
522+
:::
394523

395-
It's ok to drop the curly braces for very simple statements that fit on one line, as long as they don't have side-effects.
524+
::: {.callout-note}
525+
`ifelse(x, a, b)` is not a drop-in replacement for `if (x) a else b`. `ifelse()` is vectorised (i.e. if `length(x) > 1`, then `a` and `b` will be recycled to match) and it is eager (i.e. both `a` and `b` will always be evaluated).
526+
:::
396527

397-
```{r}
398-
# Good
399-
y <- 10
400-
x <- if (y < 20) "Too low" else "Too high"
401-
```
528+
### Control flow modifiers {#control-flow-modifiers}
402529

403-
Function calls that affect control flow (like `return()`, `stop()` or `continue`) should always go in their own `{}` block:
530+
Syntax that affects control flow (like `return()`, `stop()`, `break`, or `next`) should always go in their own `{}` block:
404531

405532
```{r}
406533
# Good
@@ -415,31 +542,22 @@ find_abs <- function(x) {
415542
x * -1
416543
}
417544
545+
for (x in xs) {
546+
if (is_done(x)) {
547+
break
548+
}
549+
}
550+
418551
# Bad
419552
if (y < 0) stop("Y is negative")
420553
421-
if (y < 0)
422-
stop("Y is negative")
423-
424554
find_abs <- function(x) {
425555
if (x > 0) return(x)
426556
x * -1
427557
}
428-
```
429-
430-
### Implicit type coercion
431-
432-
Avoid implicit type coercion (e.g. from numeric to logical) in `if` statements:
433-
434-
```{r}
435-
# Good
436-
if (length(x) > 0) {
437-
# do something
438-
}
439558
440-
# Bad
441-
if (length(x)) {
442-
# do something
559+
for (x in xs) {
560+
if (is_done(x)) break
443561
}
444562
```
445563

@@ -472,77 +590,23 @@ switch(x,
472590
switch(y, 1, 2, 3)
473591
```
474592

475-
## Long function calls
476-
477-
Strive to limit your code to 80 characters per line. This fits comfortably on a
478-
printed page with a reasonably sized font. If you find yourself running out of
479-
room, this is a good indication that you should encapsulate some of the work in
480-
a separate function.
593+
## Semicolons
481594

482-
If a function call is too long to fit on a single line, use one line each for
483-
the function name, each argument, and the closing `)`.
484-
This makes the code easier to read and to change later.
595+
Semicolons are never recommended.
596+
In particular, don't put `;` at the end of a line, and don't use `;` to put multiple commands on one line.
485597

486598
```{r}
487599
# Good
488-
do_something_very_complicated(
489-
something = "that",
490-
requires = many,
491-
arguments = "some of which may be long"
492-
)
600+
my_helper()
601+
my_other_helper()
493602
494603
# Bad
495-
do_something_very_complicated("that", requires, many, arguments,
496-
"some of which may be long"
497-
)
498-
```
499-
500-
As described under [Named arguments](#argument-names), you can omit the argument names
501-
for very common arguments (i.e. for arguments that are used in almost every
502-
invocation of the function). If this introduces a large disparity between the line lengths, you may want to supply names anyway:
604+
my_helper();
605+
my_other_helper();
503606
504-
```{r}
505-
# Good
506-
my_function(
507-
x,
508-
long_argument_name,
509-
extra_argument_a = 10,
510-
extra_argument_b = c(1, 43, 390, 210209)
511-
)
512-
513-
# Also good
514-
my_function(
515-
x = x,
516-
y = long_argument_name,
517-
extra_argument_a = 10,
518-
extra_argument_b = c(1, 43, 390, 210209)
519-
)
607+
{ my_helper(); my_other_helper() }
520608
```
521609

522-
You may place multiple unnamed arguments on the same line if they are closely
523-
related to each other. A common example of this is creating strings
524-
with `paste()`. In such cases, it's often beneficial to match one line of code
525-
to one line of output.
526-
527-
```{r}
528-
# Good
529-
paste0(
530-
"Requirement: ", requires, "\n",
531-
"Result: ", result, "\n"
532-
)
533-
534-
# Bad
535-
paste0(
536-
"Requirement: ", requires,
537-
"\n", "Result: ",
538-
result, "\n")
539-
```
540-
541-
## Semicolons
542-
543-
Don't put `;` at the end of a line, and don't use `;` to put multiple commands
544-
on one line.
545-
546610
## Assignment
547611

548612
Use `<-`, not `=`, for assignment.

0 commit comments

Comments
 (0)