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\$
4
  • \$\begingroup\$ What do you want to achieve from that rewrite? \$\endgroup\$
    – Roman Susi
    Commented Oct 8, 2017 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\$ Commented Oct 8, 2017 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
    Commented Oct 8, 2017 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
    Commented Oct 8, 2017 at 19:40

1 Answer 1

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
  • 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
    Commented Oct 9, 2017 at 15:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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