4
\$\begingroup\$

I need to find_or_create user, but first, check if his email ends with a required domain.

Can I somehow write following code using pattern matching with/case block?

  def create_user(attrs \\ %{}) do
    %User{}
    |> User.changeset(attrs)
    |> Repo.insert()
  end

  def find_or_create_user(attrs \\ %{}) do
    if String.ends_with?(attrs.email, "@test.com") do
      user = Repo.get_by(User, %{email: attrs.email}) # returns nil or user
      if user do
        {:ok, user}
      else
        create_user(attrs) # returns {:ok, user} or {:error, :reason}
      end
    else
      {:error, :invalid_email_domain}
    end
  end
\$\endgroup\$
  • \$\begingroup\$ What do you want to achieve from that rewrite? \$\endgroup\$ – Roman Susi Oct 8 '17 at 13:39
  • \$\begingroup\$ @RomanSusi to better understand elixir best practices. Coming from ruby/js background if/else seems fine to me, but every article I see with elixir code uses pattern matching. So maybe it can be written with pattern matching, but I don't know how :) \$\endgroup\$ – Ruslan Savenok Oct 8 '17 at 19:16
  • \$\begingroup\$ While I am not well-versed in elixir, I do not see how you can improve your code by using pattern matching. Pattern-matching usually helps to define a function, but here you need to do dependent checks, so if looks appropriate. You can try split user name and domain and call another function to match domain, but unless you have several domains in addition to test.com, it will just bloat your code, so I'd left find_or_create as is: Much easier to recognize be it elixir, nodejs or python. \$\endgroup\$ – Roman Susi Oct 8 '17 at 19:32
  • \$\begingroup\$ Same above applies to case: Hard to prepare cases as they are dependent on one another: Why call get_by is domain is test? \$\endgroup\$ – Roman Susi Oct 8 '17 at 19:40
3
\$\begingroup\$

I think it is a perfect fit for with. It will be something like:

def find_or_create_user(attrs \\ %{}) do
  with  true <- String.ends_with?(attrs.email, "@test.com"),
        nil <- Repo.get_by(User, %{email: attrs.email}),
        {:ok, user} <- create_user(attrs)
  do
    {:ok, user}
  else
    false -> {:error, :invalid_email_domain}
    %User{} = user -> {:ok, user}
    {:error, changeset} -> {:error, changeset}
    _anything_else -> {:error, "cannot find or create user"}
  end
end
\$\endgroup\$
  • 1
    \$\begingroup\$ I'd not called it "perfect fit", but certainly satisfies "rewrite with with" criterion. The original code is way more readable. \$\endgroup\$ – Roman Susi Oct 9 '17 at 15:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.