dealing with optional arguments via keyword list - my own take

Hi Everybody,

I spent some time testing and trying different ways how to pass, process and use optional opts in a function argument.

Ideally I’d like to settle on a single piece of code - something I can use in all my functions.

By searching I quickly realized that everybody has own take on this.

At the end I developed my own way and I’d like to kindly ask you if somebody can review it advise me if this is clever or stupid and I shouldn’t use it ;-).

defmodule Helper.ArgOpts do
  @spec add_argument(map, atom, keyword, atom) :: map
  def add_argument(map, map_key, keyword, keyword_key)
      when is_map(map) and is_atom(map_key) and is_list(keyword) and is_atom(keyword_key) do
    values =
      if Keyword.has_key?(keyword, keyword_key),
        do: Keyword.get_values(keyword, keyword_key),
        else: []

    Map.put(map, map_key, values)
  end
end

defmodule MySuperApp do
  @opts_default color: :red,
           engine: :standard
  @spec make_bike(atom, list) :: String.t()
  def make_bike(brand, opts \\ []) when is_atom(brand) and is_list(opts) do
    opts = @opts_default ++ opts
    all_opts = Enum.into(opts, %{})

	# stupid example how to use the `opts` args:
    "bike: #{all_opts.color} #{brand} with #{all_opts.engine} engine"
  end

  @opts_default number_of_wheels: 4,
           color: :red,
           engine_size: :standard,
           tuning?: false
  @spec make_car(atom, list) :: String.t()
  def make_car(brand, opts \\ []) when is_atom(brand) and is_list(opts) do
    opts = @opts_default ++ opts
    all_opts = Enum.into(opts, %{})
    all_opts = Helper.ArgOpts.add_argument(all_opts, :features, opts, :feature)

	# stupid example how to use the `opts` args:
    engine =
      if all_opts.tuning?,
        do: "TUNED engine",
        else: "#{all_opts.engine_size} engine"

    """
    car: #{all_opts.color} #{brand}
      with #{all_opts.number_of_wheels} wheels
      and #{engine}
      and features: #{inspect(all_opts.features)}
    """
  end
end

IO.puts(MySuperApp.make_bike(:honda))

IO.puts(MySuperApp.make_bike(:kawasaki, engine: :fastest, color: :green))

IO.puts(MySuperApp.make_car(:ferrari))

IO.puts(MySuperApp.make_car(:lamborghini, color: :yellow, feature: :air_con, feature: :shiny_wheels))

IO.puts(MySuperApp.make_car(:hummer, color: :black, number_of_wheels: 6, tuning?: true, feature: :dark_window_tint))

Basically the idea is to have an attribute @opts_default with default values on top of each function that has opts argument.

In a simple functions (like make_bike/2) the Keyword list with defaults + the keyword list with user specified values is converted into a Map. Defaults get overwritten by user specified values.

In more complicated functions (like make_car/2) it is possible to repeat a keyword (in the example feature) and then add a features key into the new map. New features key holds all values (or empty list if feature is not specified).

Can I please get feedback if this is/isn’t a good way.

Thank you ;-).

Kind regards,

Ben

I am confused as to why you wouldn’t use the Keyword module and its functions for that purpose.

You can easily merge the options provided by the user and the defaults with Keyword.merge/2 eg :

opts = Keyword.merge(@default_opts, opts)

You can also gather duplicated keys with Keyword.get_values/2 or, closer to your example Keyword.pop_values/2 eg :

 @opts_default number_of_wheels: 4,
           color: :red,
           engine_size: :standard,
           tuning?: false
  @spec make_car(atom, list) :: String.t()
  def make_car(brand, opts \\ []) when is_atom(brand) and is_list(opts) do
    opts = Keyword.merge(@opts_default, opts)
    {features, opts } = Keyword.pop_values(opts, :feature)
    all_opts = Enum.into(opts, %{}) |> Map.put(:features, features)

	# stupid example how to use the `opts` args:
    engine =
      if all_opts.tuning?,
        do: "TUNED engine",
        else: "#{all_opts.engine_size} engine"

    """
    car: #{all_opts.color} #{brand}
      with #{all_opts.number_of_wheels} wheels
      and #{engine}
      and features: #{inspect(all_opts.features)}
    """
  end
1 Like

Hi krstfk,

Keyword.merge/2 keeps only a single key of the same name.

iex(1)> Keyword.merge([a: 22, b: 33], [a: 33, d: 44])
[b: 33, a: 33, d: 44]

Rest of the code is cool, but I’d probably move it into a separate function since I’ll use it a lot.

Modified version (only the helper changed):

defmodule Helper.ArgOpts do
  @spec add_argument(map, atom, keyword, atom) :: map
  def add_argument(map, map_key, keyword, keyword_key)
      when is_map(map) and is_atom(map_key) and is_list(keyword) and is_atom(keyword_key) do
    {values, _} = Keyword.pop_values(keyword, keyword_key)
    Map.put(map, map_key, values)
  end
end

defmodule MySuperApp do
  @opts_df color: :red,
           engine: :standard
  @spec make_bike(atom, list) :: String.t()
  def make_bike(brand, opts \\ []) when is_atom(brand) and is_list(opts) do
    opts = @opts_df ++ opts
    all_opts = Enum.into(opts, %{})

    # IO.inspect(all_opts)
    # IO.puts("tuning: #{inspect(all_opts.tuning?)}")

    "bike: #{all_opts.color} #{brand} with #{all_opts.engine} engine"
  end

  @opts_df number_of_wheels: 4,
           color: :red,
           engine_size: :standard,
           tuning?: false
  @spec make_car(atom, list) :: String.t()
  def make_car(brand, opts \\ []) when is_atom(brand) and is_list(opts) do
    opts = @opts_df ++ opts
    all_opts = Enum.into(opts, %{})
    all_opts = Helper.ArgOpts.add_argument(all_opts, :features, opts, :feature)

    # IO.inspect(all_opts)
    # IO.puts("tuning: #{inspect(all_opts.tuning?)}")

    engine =
      if all_opts.tuning?,
        do: "TUNED engine",
        else: "#{all_opts.engine_size} engine"

    """
    car: #{all_opts.color} #{brand}
      with #{all_opts.number_of_wheels} wheels
      and #{engine}
      and features: #{inspect(all_opts.features)}
    """
  end
end

I thought that is what you wanted, since you want to give the user the ability to override the defaults options, but duplicate keys are handled in sensible way by Keyword.merge/2 :

iex(1)> opts = [a: 1, a: 2, b: 3]
[a: 1, a: 2, b: 3]
iex(2)> opts2 = [a: 3, a: 4, a: 5, b: 3]
[a: 3, a: 4, a: 5, b: 3]
iex(3)> Keyword.merge(opts, opts2)
[a: 3, a: 4, a: 5, b: 3]

If you want some keys from the first argument to be kept no matter what you can pop them before merging with eg Keyword.split/2 or Keyword.pop_values/2 and put them back :

iex(11)> opts = [feature: 1, feature: 2, option1: 3, option2: 4]
[feature: 1, feature: 2, option1: 3, option2: 4]
iex(12)> opts2 = [feature: 3, feature: 4, option1: 5]
[feature: 3, feature: 4, option1: 5]
iex(13)> {features, opts} = Keyword.split(opts, [:feature])
{[feature: 1, feature: 2], [option1: 3, option2: 4]}
iex(14)> opts =  features ++  Keyword.merge(opts, opts2)
[feature: 1, feature: 2, option2: 4, feature: 3, feature: 4, option1: 5]

I usually just do:

color = opts[:color] || @default_opts[:color]

for every single opt that I care. it is the same as Keyword.merge/2 but explicit, you can add any sanity check you want, and you end up with local bindings that are easier to use.

1 Like

Keyword.get/3 accepts a default value, which would work for false and nil values, while your version would (surprisingly) overwrite them.

color = Keyword.get(opts, :color, @default_opts[:color])

Example:

opts = {color: false}

opts[:color] || true
#⇒ true

Keyword.get(opts, :color, true)
#⇒ false
1 Like

Of course I was assuming that the opts were not legitimately false or nil. for those you need to do like you said.

I would also consider passing optional arugument as nil is bad style. I am not sure what that is supposed to mean. OTOH false does make some sense; however if something defaults to true but can be turned off by options we can simply use && instead of ||:

turbo? = opts[:turbo?] && @default_opts[:turbo?]

It won’t work, unfortunately. nil && default would result in nil.

E.g. the logger. nil is none, while default is :console.

1 Like

It won’t, as nil is considered “falsy”:

iex(1)> nil || "foo"
"foo"

Argh, I messed || with &&, there should be &&; check the comment I responded to.

I see. You are absolutely correct.

  • Boolean options should not use || or && to decode.
  • Options that can be nil should not use || to decode