Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument

Question

I am working on my rails RESTful API and I have set up a versioning feature on some of the endpoints. I have a class ApiVersion which Is responsible for determining which controllers to render base on the arguments passed to it on initialization.

The class definition looks as follows:

 class ApiVersion
  attr_reader :version, :default

  def initialize(version, default = false)
    @version = version
    @default = default
  end

  # check whether version is specified or is default
  def matches?(request)
    check_headers(request.headers) || default
  end

  private

  def check_headers(headers)
    # check version from Accept headers; expect custom media type 'suits'
    accept = headers[:accept]
    accept&.include?("application/vnd.suits.#{version}+json")
  end
end

The requests work perfectly fine but when I run rubocop -A I get an error that says:

 Style/OptionalBooleanParameter: Use keyword arguments when defining method with boolean argument.
  def initialize(version, default = false)

I searched on the internet how to fix this type of error & got some interesting ideas which could not work in my case. For example I found one post that said I should alternate the def initialize(version, default = false) with def initialize(version, default: false) which passes the rubocop tests but then I get an internal server error with an exception: ArgumentError: wrong number of arguments (given 2, expected 1).

Does anyone have an idea on how I can fix this, or how I can alternate the class definition, to get around this issue? Thank you

Answer 1

First off: if you disagree with a particular rule in a linter, then turn it off. In particular, this rule is in the "Style" category, so it is not a correctness or security issue, it is a matter of style.

Secondly, boolean parameters are a code smell, since they are often Flag Parameters. A method with a flag parameter will generally do two different things depending on the value of the boolean argument, because … why else would it have the flag parameter?

However, a method that does two different things should probably be two methods.

Or, in this particular case, since it is an object initializer method specifically, that hints at the fact that there should be two classes.

Okay, with that out of the way, the nice thing about Rubocop is that it generally tells you how to fix whatever it is complaining about. In this case, it suggests using a keyword parameter. That doesn't fix the problem that the method is likely still doing two different things, but at least, it gives a name to that difference, so you can see it at the call site.

So, what Rubocop is suggest is to change the positional parameter into a keyword parameter, something like this:

def initialize(version, default: false)

Now, obviously, when you change the parameter list at the definition site, you also need to change every argument list at every call site. So, if you have a call like this (remember that #initialize gets called by ::new):

ApiVersion.new('1.2.3', true)

You need to replace it with

ApiVersion.new('1.2.3', default: true)