Should I set map key's value as return value of function when defining map? Or pass entire map to another function to set that key value pair?

So this is kind of a general best practice question. I have this example demonstrating the problem (please don’t get too hung up on the details behind this example, this is just something I made up to demonstrate my question)

Assume user is a map, which contains a name (required) and a nickname (might be nil)

User has been passed as an argument to another function. Inside this function, a new map is being created. A property on this new map, is dependent upon whether user has a nickname value that is not nil.

Which of the following two examples is the preferred way for handling this?

Option 1

 %{
  name: determine_name(user),
  favorite_food: "pizza"
 }

then having determine_name/1 function

defp determine_name(%{nickname: nil, name: name}), do: name
defp determine_name(%{nickname: nickname, name: name}), do: "#{name} (#{nickname})"

Option 2

%{
  favorite_food: "pizza"
}
|> put_name(user)

and having put_name/2 defined

defp put_name(food_map, %{nickname: nil, name: name}), do: Map.put(:name, name)
defp put_name(food_map, %{nickname: nickname, name: name}), do: Map.put(:name, "#{name} (#{nickname})")

OR another option…

I ask because I have instances of both in my code.

  • I use option 1. in a case where I need to do something with a time or date
  • I tend to use option 2. in a situation like the one described above

Let me know your thoughts! Thanks!

I’d usually recommend option 3: make a User struct and move determine_name into the struct’s module.

FWIW, if I needed to change “option 2”-shaped code I’d usually refactor it to “option 1”. IMO determine_name does a better job of separating “creating the result” from “putting the result in the Map”.

1 Like

yeah, option 2 seems less explicit, what’s actually going on is hidden for no reason

1 Like