3
votes

I have the following function definitions, and I'm trying to find a way to simplify the params so that they're all uniform.

  def update_user(%User{} = user, %{password: _} = attrs) do
    attrs = Map.put(attrs, :password_reset_token, nil)
    update_user(user, attrs, &User.password_changeset/2)
  end

  def update_user(%User{} = user, %{password_reset_token: _} = attrs), do:
    update_user(user, attrs, &User.password_reset_changeset/2)

  def update_user(user, attrs), do:
    update_user(user, attrs, &User.changeset/2)

  def update_user(user, attrs, changeset) do
    user
    |> changeset.(attrs)
    |> Repo.update()
  end

A programmer could mess that up I think, by passing in a map with string keys (say for %{"password" => value}) and it'd match with update_user/2. So maybe I should update the params for those functions? Or use a guard on update_user(user, attrs) to not match w/ %{"password" => value}?

Contrast with the following, which takes params straight from a controller function (def create(conn, params)):

  def authenticate_user(%{"email" => email, "access_token" => _}) do
    case get_by(%{email: email}) do
      %User{} = user ->
        {:ok, user}
      nil ->
        {:error, :non_existent}
    end
  end

  def authenticate_user(%{"email" => email, "password" => password}) do
    with %User{} = user <- get_by(%{email: email}),
      {:ok} <- verify_password(password, user.password_hash),
      do: {:ok, user}
  end

Those params are matching with a map that has string keys. As I mentioned I'm doing that because then I can pass params straight from the controller and pattern match with what's been passed in (in this case, various login methods). Is there a standard Phoenix way for tackling this kind of problem?

1
One way would be to convert all atom keys to string keys and then do the pattern matching only on strings. Another way is to use cond, e.g. cond do attrs["password"] || attrs[:password] -> # password exists .... I'd rather just call the function with string keys so only a single pattern match is needed (since you cannot change the behavior of params having string keys). - Dogbert

1 Answers

0
votes

Here is one possible solution using pattern matching.

It might not the most beautiful solution, but I don't think matching for both string and atom keys ever is.

  def update_user(%User{} = user, attrs) do
    user
    |> user_changeset(attrs)
    |> Repo.update()
  end

  def user_changeset(user, attrs) do
    case attrs do
      %{"password" => _} ->
        User.password_changeset(user, Map.put(attrs, "password_reset_token", nil))

      %{password: _} ->
        User.password_changeset(user, Map.put(attrs, :password_reset_token, nil))

      %{"password_reset_token" => _} ->
        User.password_reset_changeset(user, attrs)

      %{password_reset_token: _} ->
        User.password_reset_changeset(user, attrs)

      _ ->
        User.changeset(user, attrs)
    end
  end

Also, you could pattern match the function arguments for all cases, but I think that's messier in this case.

Another option would be to first always transform all attrs maps into string keys (the reverse, dynamically generating atoms is generally considered a bad idea).