tidyverse / style

The tidyverse style guide for R code
https://style.tidyverse.org
Other
298 stars 104 forks source link

Improve advice for unnamed arguments on long lines #189

Closed hadley closed 1 month ago

hadley commented 2 years ago

The style guide has:

map(x, f,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)

But I think I now prefer keeping each argument on its own line:

map( 
  x, 
  f,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)

It only looks a bit weird here because the input names are so short.

lorenzwalthert commented 2 years ago

It only looks a bit weird here because the input names are so short.

I agree. I interpret the current rules as

# not too long
map(address, extract_house_number,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)

# too long
map(
  mutual_friends$address, 
  extract_house_number_from_address,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)

So are there any circumstances under which you want to allow for the current practice and leave

map(x, f,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)

as is?

hadley commented 2 years ago

I don't think so, it now feels pretty idiosyncratic to me.

At a minimum, it feels like it should be:

map(
  x, f,
  extra_argument_a = 10,
  extra_argument_b = c(1, 43, 390, 210209)
)
MichaelChirico commented 2 years ago

Definitely think there are a lot of cases where arguments belong on the same line together (paste is the simplest example), but I agree that bumping everything to the next line almost always looks better.

sprintf for me is more like Lorenz' example -- group the args if they fit on one line, individual lines otherwise:

sprintf(
  "formatting string %s%s%s%s",
  a, b, c, d
)

sprintf(
  "formatting string %s%s%s%s",
  longargument1,
  longargument2,
  longargument3,
  longargument4
)
hadley commented 1 month ago

I think what I was originally reacting to is that code like this looks aesthetically displeasing to me:

f(
  x,
  very_long_argument, 
  y.
  long_argument_name = long_argument_value
)

But I don't think that avoiding that problem by folding several short arguments into a line makes sense as general principle.

MichaelChirico commented 1 month ago

My preference for cases like this is to name the shorter arguments if possible, e.g. this comes up a lot with round()/apply():

round(a_really_long_expression(x, y)[[2]], 1)
# if split across lines becomes
round(
  a_really_long_expression(x, y)[[2]],
  digits = 1
)
# vs IMO less readable
round(
  a_really_long_expression(x, y)[[2]],
  1
)

apply(x, 1, function(xi) a_really_long_expression(xi))
# if split across lines becomes
apply(
  x,
  MARGIN = 1,
  function(xi) a_really_long_expression(xi)
)
# vs IMO less readable
apply(
  x,
  1,
  function(xi) a_really_long_expression(xi)
)

The two cases to keep in mind to avoid over-specifying the rule (IMO) are base::plot() and paste(), both of which I think are most readable when grouping arguments in some way.

The former because we group named arguments for clarity, the latter because we group unnamed (and unnameable!) arguments for clarity.