ruby-style-guide icon indicating copy to clipboard operation
ruby-style-guide copied to clipboard

Parameter Ordering

Open norswap opened this issue 8 years ago • 10 comments

I feel the guide should have a say on how to order method parameters. In particular, default parameter should occur after non-default regular parameters, and before the splat parameter.

Doing things differently makes it quite difficult to reason out of which arguments are assigned to which parameter, especially when default parameters are mixed with the splat parameter.

In fact, mixing default parameters with the splat parameter should be avoided: it's not quite obvious that arguments go to the default parameters rather than to the the splat!

(Much) more details here: http://norswap.com/ruby-methods/

Here's an example to illustrate the non-obviousness:

def foo a, b=42, c *d, e
    p [a, b, c, d, e]
end

foo 1, 2, 3     # a=1, b=42, c=2, d=[], e=3
foo 1, 2, 3, 4  # a=1, b=2, c=3, d=[], e=4

Personally, I would go even further and say to avoid default parameters entirely in favour of keyword parameters with default value, making everything nice and regular. But that's perhaps venturing too far in the realm of personal preferences. Your call.

norswap avatar Feb 09 '17 00:02 norswap

This is an invalid Ruby code.

andreydeineko avatar May 08 '18 20:05 andreydeineko

I'd welcome improvements in this area. I thought we already had some rules that encourage the use of keyword params, but we can always do more.

bbatsov avatar Jun 12 '19 15:06 bbatsov

We have https://github.com/rubocop-hq/ruby-style-guide/#optional-arguments

Define optional arguments at the end of the list of arguments.

This is an invalid Ruby code.

Indeed, but:

def foo(a, b=42, c)
  # ...
end

and

def f2(a, b=42, *c)
  # ...
end

are valid.

Not sure if it makes sense to add a guideline to put splat parameters before ordinary ones. Is there any problem with:

def assign_kids_to_school(*kids, school)
  # ...
end

assign_kids_to_school(john, mary, penny, vicky, school)

? Is the following preferrable, and why?

def assign_kids_to_school(school, *kids)
  # ...
end

assign_kids_to_school(school, john, mary, penny, vicky)

I clearly understand that assign_kids_to_school(john, mary, penny, vicky, school: best_school_in_the_world) is better, but let's set kwargs aside in this discussion.

pirj avatar Feb 24 '21 16:02 pirj

I clearly understand that assign_kids_to_school(john, mary, penny, vicky, school: best_school_in_the_world) is better,

The fact that is it so much better is the reason that def assign_kids_to_school(*kids, school) should be clearly discouraged.

My personal rule of thumb is that public methods should have at most one "kind" of unnamed argument. Everything else should be kwargs.

marcandre avatar Feb 24 '21 16:02 marcandre

Does this cover the rule of thumb, or would you add something, or would like to extend it to be more clear, @marcandre ?

It seems that there's nothing actionable left here otherwise.

pirj avatar Feb 24 '21 16:02 pirj

I don't like the current "Optional Arguments" section.

  1. It is contradicted by the later rule you point out
  2. "Ruby has some unexpected results when calling methods that have optional arguments at the front of the list" => AFAIK is flat out wrong, especially in 3.0 with improved handling of keyword parameters.

Could we not remove it altogether?

marcandre avatar Feb 24 '21 16:02 marcandre

"Prefer keyword arguments" in "Keyword Arguments vs Optional Arguments" is a soft recommendation. Some would still prefer using optional positional arguments and "Optional Arguments" section will still be quite useful for them.

pirj avatar Feb 24 '21 17:02 pirj

Can we make is a hard recommendation, and remove the "Optional arguments" then? 😅 Or at least remove the "Ruby has some unexpected results when calling methods that have optional arguments at the front of the list"...

marcandre avatar Feb 24 '21 17:02 marcandre

Can we make is a hard recommendation, and remove the "Optional arguments" then?

🤷 If we'd move to a three-grade system similar to RFC2119 like #521 suggests, I'm all for increasing the requirement level from the current "prefer" (MAY/OPTIONAL) to SHOULD, but probably not MUST.

remove the "Ruby has some unexpected results

some_method('w', 'x') # => '1, 2, w, x'
some_method('w', 'x', 'y') # => 'w, 2, x, y'

and

def foo(a, b=42, *c)
  [a, b, *c].join(', ')
end

foo(1, 2) # => "1, 2"
foo(1) # => "1, 42"

are expected results for an experienced developer. But not for me 😄

There's not much logic and reasoning in this 👇 paragraph, just the exposure of my doubts, fear and uncertainty Personally, I'm not too fond of keyword arguments just yet due to its forwarding deficiencies in the current Ruby 3.0, e.g.:

# sorry for a pretty contrived example
def build_a_timeline(...)
  build_a_timeline(..., Time.current) # SyntaxError: unexpected ',', expecting ')'
end

# yet another
def build_a_timeline(..., limit: 10) # SyntaxError: unexpected ',', expecting ')'

There are probably better approaches in each scenario.

pirj avatar Feb 24 '21 17:02 pirj

I'm not too fond of keyword arguments

Sorry to hear how much we disagree on this

forwarding deficiencies [with ...]

Glad to see we agree on this, for .... Explicit *args, **options works fine though.

marcandre avatar Feb 24 '21 18:02 marcandre