From 389d607e83e8ac6795d9b4f439ad6c90e350b324 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Fri, 14 Feb 2025 12:43:17 -0500 Subject: [PATCH 1/8] Adjust rule for single line vs multiline if statements --- functions.qmd | 2 +- syntax.qmd | 64 ++++++++++++++++++++++++++++++--------------------- 2 files changed, 39 insertions(+), 27 deletions(-) diff --git a/functions.qmd b/functions.qmd index e8812bc..6a60524 100755 --- a/functions.qmd +++ b/functions.qmd @@ -154,7 +154,7 @@ add_two <- function(x, y) { } ``` -Return statements should always be on their own line because they have important effects on the control flow. See also [inline statements](#inline-statements). +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). ```{r} # Good diff --git a/syntax.qmd b/syntax.qmd index 73aa6d7..163b44f 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -297,7 +297,7 @@ output <- capture.output(x <- f()) ### Code blocks {#indenting} -Curly braces, `{}`, define the most important hierarchy of R code. To make this +Braced expressions, `{}`, define the most important hierarchy of R code. To make this hierarchy easy to see: * `{` should be the last character on the line. Related code (e.g., an `if` @@ -308,7 +308,7 @@ hierarchy easy to see: * `}` should be the first character on the line. -It is occassionally useful to have an empty curly braces block, in which case it should be written `{}`. +It is occassionally useful to have empty braced expressions, in which case it should be written `{}`. ```{r} # Good @@ -364,41 +364,56 @@ while (waiting_for_something()) { ### If statements -* When present, `else` should be on the same line as `}`. +* A single line if statement must never contain braced expressions. You can use + single line if statements for very simple statements that don't have + side-effects and don't modify the control flow. -* `&` and `|` should never be used inside of an `if` clause because they can - return vectors. Always use `&&` and `||` instead. + ```{r} + # Good + message <- if (x > 10) "big" else "small" -* NB: `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). + # Bad + message <- if (x > 10) { "big" } else { "small" } + ``` - If you want to rewrite a simple but lengthy `if` block: +* A multiline if statement must contain braced expressions. ```{r} + # Good + if (x > 10) { + x * 2 + } + if (x > 10) { - message <- "big" + x * 2 } else { - message <- "small" + x * 3 } - ``` - Just write it all on one line: + # Bad + if (x > 10) + x * 2 - ```{r} - message <- if (x > 10) "big" else "small" + # In particular, the outer braces are required for this to even parse. + { + if (x > 10) + x * 2 + else + x * 3 + } ``` -### Inline statements {#inline-statements} +* When present, `else` should be on the same line as `}`. -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. +* `&` and `|` should never be used inside of an `if` clause because they can + return vectors. Always use `&&` and `||` instead. -```{r} -# Good -y <- 10 -x <- if (y < 20) "Too low" else "Too high" -``` +* NB: `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). + +### Control flow modifiers {#control-flow-modifiers} Function calls that affect control flow (like `return()`, `stop()` or `continue`) should always go in their own `{}` block: @@ -418,9 +433,6 @@ find_abs <- function(x) { # Bad if (y < 0) stop("Y is negative") -if (y < 0) - stop("Y is negative") - find_abs <- function(x) { if (x > 0) return(x) x * -1 From 1313680406832d501649a0672600ca017fc86225 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 11:13:10 -0500 Subject: [PATCH 2/8] Respond to feedback, and pull out Braced Expressions into its own section --- syntax.qmd | 93 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 71 insertions(+), 22 deletions(-) diff --git a/syntax.qmd b/syntax.qmd index 163b44f..cd2266a 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -292,24 +292,19 @@ The only exception is in functions that capture side-effects: output <- capture.output(x <- f()) ``` +## Braced expressions {#braced-expressions} -## Control flow - -### Code blocks {#indenting} +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 function calls (such as `tryCatch()` or `test_that()` blocks). -Braced expressions, `{}`, define the most important hierarchy of R code. To make this -hierarchy easy to see: +To make this hierarchy easy to see: -* `{` should be the last character on the line. Related code (e.g., an `if` - clause, a function declaration, a trailing comma, ...) must be on the same - line as the opening brace. +* `{` should be the last character on the line. + Related code (e.g., an `if` clause, a function declaration, a trailing comma, ...) must be on the same line as the opening brace. * The contents should be indented by two spaces. * `}` should be the first character on the line. -It is occassionally useful to have empty braced expressions, in which case it should be written `{}`. - ```{r} # Good if (y < 0 && debug) { @@ -340,8 +335,6 @@ tryCatch( } ) -while (waiting_for_something()) {} - # Bad if (y < 0 && debug) { message("Y is negative") @@ -355,13 +348,58 @@ if (y == 0) message("x is negative or zero") } } else { y ^ x } +``` -while (waiting_for_something()) { } -while (waiting_for_something()) { +It is occasionally useful to have empty braced expressions, in which case it should be written `{}`, with no intervening space. + +```{r} +# Good +function(...) {} + +# Bad +function(...) { } +function(...) { } ``` +## Control flow + +### Loops + +R defines three types of looping constructs: `for`, `while`, and `repeat` loops. + +* The body of a loop must be a braced expression. + + ```{r} + # Good + for (i in seq) { + x[i] <- x[i] + 1 + } + + while (waiting_for_something()) { + cat("Still waiting...") + } + + # Bad + for (i in seq) x[i] <- x[i] + 1 + + while (waiting_for_something()) cat("Still waiting...") + ``` + +* 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 `{}`. + + ```{r} + # Good + while (waiting_for_something()) {} + + # Bad + while (waiting_for_something()) { } + while (waiting_for_something()) { + + } + ``` + ### If statements * A single line if statement must never contain braced expressions. You can use @@ -394,7 +432,8 @@ while (waiting_for_something()) { if (x > 10) x * 2 - # In particular, the outer braces are required for this to even parse. + # In particular, this if statement will only parse when wrapped in a braced + # expression or call { if (x > 10) x * 2 @@ -405,17 +444,17 @@ while (waiting_for_something()) { * When present, `else` should be on the same line as `}`. -* `&` and `|` should never be used inside of an `if` clause because they can - return vectors. Always use `&&` and `||` instead. +::: {.callout-note} +`&` and `|` should never be used inside of an `if` clause because they can return vectors. Always use `&&` and `||` instead. +::: -* NB: `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). +::: {.callout-note} +`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). +::: ### Control flow modifiers {#control-flow-modifiers} -Function calls that affect control flow (like `return()`, `stop()` or `continue`) should always go in their own `{}` block: +Syntax that affects control flow (like `return()`, `stop()`, `break`, or `next`) should always go in their own `{}` block: ```{r} # Good @@ -430,6 +469,12 @@ find_abs <- function(x) { x * -1 } +for (x in xs) { + if (is_done(x)) { + break + } +} + # Bad if (y < 0) stop("Y is negative") @@ -437,6 +482,10 @@ find_abs <- function(x) { if (x > 0) return(x) x * -1 } + +for (x in xs) { + if (is_done(x)) break +} ``` ### Implicit type coercion From 8f4d58a60beb27edf48ad26b0458e3288316431b Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 11:17:39 -0500 Subject: [PATCH 3/8] Move `Long function calls` under `Function calls` --- syntax.qmd | 132 ++++++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 66 deletions(-) diff --git a/syntax.qmd b/syntax.qmd index cd2266a..62405a7 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -292,6 +292,72 @@ The only exception is in functions that capture side-effects: output <- capture.output(x <- f()) ``` +### Long function calls + +Strive to limit your code to 80 characters per line. This fits comfortably on a +printed page with a reasonably sized font. If you find yourself running out of +room, this is a good indication that you should encapsulate some of the work in +a separate function. + +If a function call is too long to fit on a single line, use one line each for +the function name, each argument, and the closing `)`. +This makes the code easier to read and to change later. + +```{r} +# Good +do_something_very_complicated( + something = "that", + requires = many, + arguments = "some of which may be long" +) + +# Bad +do_something_very_complicated("that", requires, many, arguments, + "some of which may be long" + ) +``` + +As described under [Named arguments](#argument-names), you can omit the argument names +for very common arguments (i.e. for arguments that are used in almost every +invocation of the function). If this introduces a large disparity between the line lengths, you may want to supply names anyway: + +```{r} +# Good +my_function( + x, + long_argument_name, + extra_argument_a = 10, + extra_argument_b = c(1, 43, 390, 210209) +) + +# Also good +my_function( + x = x, + y = long_argument_name, + extra_argument_a = 10, + extra_argument_b = c(1, 43, 390, 210209) +) +``` + +You may place multiple unnamed arguments on the same line if they are closely +related to each other. A common example of this is creating strings +with `paste()`. In such cases, it's often beneficial to match one line of code +to one line of output. + +```{r} +# Good +paste0( + "Requirement: ", requires, "\n", + "Result: ", result, "\n" +) + +# Bad +paste0( + "Requirement: ", requires, + "\n", "Result: ", + result, "\n") +``` + ## Braced expressions {#braced-expressions} 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 function calls (such as `tryCatch()` or `test_that()` blocks). @@ -533,72 +599,6 @@ switch(x, switch(y, 1, 2, 3) ``` -## Long function calls - -Strive to limit your code to 80 characters per line. This fits comfortably on a -printed page with a reasonably sized font. If you find yourself running out of -room, this is a good indication that you should encapsulate some of the work in -a separate function. - -If a function call is too long to fit on a single line, use one line each for -the function name, each argument, and the closing `)`. -This makes the code easier to read and to change later. - -```{r} -# Good -do_something_very_complicated( - something = "that", - requires = many, - arguments = "some of which may be long" -) - -# Bad -do_something_very_complicated("that", requires, many, arguments, - "some of which may be long" - ) -``` - -As described under [Named arguments](#argument-names), you can omit the argument names -for very common arguments (i.e. for arguments that are used in almost every -invocation of the function). If this introduces a large disparity between the line lengths, you may want to supply names anyway: - -```{r} -# Good -my_function( - x, - long_argument_name, - extra_argument_a = 10, - extra_argument_b = c(1, 43, 390, 210209) -) - -# Also good -my_function( - x = x, - y = long_argument_name, - extra_argument_a = 10, - extra_argument_b = c(1, 43, 390, 210209) -) -``` - -You may place multiple unnamed arguments on the same line if they are closely -related to each other. A common example of this is creating strings -with `paste()`. In such cases, it's often beneficial to match one line of code -to one line of output. - -```{r} -# Good -paste0( - "Requirement: ", requires, "\n", - "Result: ", result, "\n" -) - -# Bad -paste0( - "Requirement: ", requires, - "\n", "Result: ", - result, "\n") -``` - ## Semicolons Don't put `;` at the end of a line, and don't use `;` to put multiple commands From df4df4c20306f1c5f3c285d615e15ccc0b98743d Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 11:19:08 -0500 Subject: [PATCH 4/8] Move implicit type coercion into if statement section --- syntax.qmd | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/syntax.qmd b/syntax.qmd index 62405a7..bca31db 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -510,6 +510,20 @@ R defines three types of looping constructs: `for`, `while`, and `repeat` loops. * When present, `else` should be on the same line as `}`. +* Avoid implicit type coercion (e.g. from numeric to logical) in the condition of an if statement: + + ```{r} + # Good + if (length(x) > 0) { + # do something + } + + # Bad + if (length(x)) { + # do something + } + ``` + ::: {.callout-note} `&` and `|` should never be used inside of an `if` clause because they can return vectors. Always use `&&` and `||` instead. ::: @@ -554,22 +568,6 @@ for (x in xs) { } ``` -### Implicit type coercion - -Avoid implicit type coercion (e.g. from numeric to logical) in `if` statements: - -```{r} -# Good -if (length(x) > 0) { - # do something -} - -# Bad -if (length(x)) { - # do something -} -``` - ### Switch statements * Avoid position-based `switch()` statements (i.e. prefer names). From 0cef0ecb388765c03dad6aa5d79d42bc61219226 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 11:22:42 -0500 Subject: [PATCH 5/8] Expand semicolon section a bit --- syntax.qmd | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/syntax.qmd b/syntax.qmd index bca31db..8c8c916 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -599,8 +599,20 @@ switch(y, 1, 2, 3) ## Semicolons -Don't put `;` at the end of a line, and don't use `;` to put multiple commands -on one line. +Semicolons are never recommended. +In particular, don't put `;` at the end of a line, and don't use `;` to put multiple commands on one line. + +```{r} +# Good +my_helper() +my_other_helper() + +# Bad +my_helper(); +my_other_helper(); + +{ my_helper(); my_other_helper() } +``` ## Assignment From 465f6683b53a283b2b7c8cb0ae52d55ab2475eab Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 11:38:42 -0500 Subject: [PATCH 6/8] Update syntax.qmd Co-authored-by: Lionel Henry --- syntax.qmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syntax.qmd b/syntax.qmd index 8c8c916..2653a8d 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -297,7 +297,7 @@ output <- capture.output(x <- f()) Strive to limit your code to 80 characters per line. This fits comfortably on a printed page with a reasonably sized font. If you find yourself running out of room, this is a good indication that you should encapsulate some of the work in -a separate function. +a separate function or use early returns to reduce the nesting in your code. If a function call is too long to fit on a single line, use one line each for the function name, each argument, and the closing `)`. From 063d85355aa88ac636965957023edad0122ae584 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 16:36:08 -0500 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Hadley Wickham --- syntax.qmd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syntax.qmd b/syntax.qmd index 2653a8d..25ece0e 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -360,7 +360,7 @@ paste0( ## Braced expressions {#braced-expressions} -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 function calls (such as `tryCatch()` or `test_that()` blocks). +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()`). To make this hierarchy easy to see: From 19a4084e46b7a6bab19e6ca67ae8b7b1f93946d5 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 18 Feb 2025 16:38:59 -0500 Subject: [PATCH 8/8] Apply code review suggestions --- syntax.qmd | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/syntax.qmd b/syntax.qmd index 25ece0e..3cde2cc 100755 --- a/syntax.qmd +++ b/syntax.qmd @@ -455,17 +455,6 @@ R defines three types of looping constructs: `for`, `while`, and `repeat` loops. * 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 `{}`. - ```{r} - # Good - while (waiting_for_something()) {} - - # Bad - while (waiting_for_something()) { } - while (waiting_for_something()) { - - } - ``` - ### If statements * A single line if statement must never contain braced expressions. You can use @@ -478,6 +467,10 @@ R defines three types of looping constructs: `for`, `while`, and `repeat` loops. # Bad message <- if (x > 10) { "big" } else { "small" } + + if (x > 0) message <- "big" else message <- "small" + + if (x > 0) return(x) ``` * A multiline if statement must contain braced expressions.