Since you can’t do early returns as others have mentioned you need to define two (in this case) branches of execution. You can use if, but I find case to be more readable usually:
def create(conn, body) do
case PayeeContext.check_for_required_fields(body) do
false -> send_resp(conn, 400, "Please provide Key and Type parameters")
_ ->
%{"key" => key, "type" => type} = body
case PayeeContext.validation_map(type).(key) do
{:ok, _} -> send_resp(conn, 200, "Request created successfully")
{:error, message} -> send_resp(conn, 400, message)
end
end
end
If those are the only keys you need (“key” and “type” and check_for_required_fields doesn’t do anything else) then you can use pattern matching on the function head to single out the missing case:
def create(conn, %{"key" => key, "type" => type} = body) do
# we have both keys for sure
case PayeeContext.validation_map(type).(key) do
{:ok, _} -> send_resp(conn, 200, "Request created successfully")
{:error, message} -> send_resp(conn, 400, message)
end
end
def create(conn, _), do: send_resp(conn, 400, "Please provide Key and Type parameters")
Although not related directly to your question, I think that probably the best solution would be to implement a schema with Ecto and use that to validate the params itself. This would make it easier to return the exact missing/invalid parameters, as you can rely on the changeset API and in my opinion is cleaner. An example would be:
defmodule Checkout.Request.Type do
@behaviour Ecto.Type
@type t() :: :one_type_request | :another_type_request
def type, do: :string
@valid_types [:one_type_request, :another_type_request]
@valid_string Enum.reduce(@valid_types, [], fn(t, acc) -> [Atom.to_string(t) | acc] end)
@valid_map Enum.reduce(@valid_types, %{}, fn(t, acc) -> Map.put(acc, Atom.to_string(t), t) end)
def valid_types(), do: @valid_types
def load(data) when is_binary(data) and data in @valid_string, do: {:ok, @valid_map[data]}
def load(data) when is_atom(data) and data in @valid_types, do: {:ok, data}
def load(_), do: :error
def cast(data) when is_atom(data) and data in @valid_types, do: {:ok, data}
def cast(data) when is_binary(data) and data in @valid_string, do: {:ok, String.to_atom(data)}
def cast(_), do: :error
def dump(data) when is_atom(data) and data in @valid_types, do: {:ok, Atom.to_string(data)}
def dump(data) when is_binary(data) and data in @valid_string, do: {:ok, data}
def embed_as(_), do: :dump
def equal?(term_1, term_2) when is_binary(term_1) and term_1 in @valid_string and is_atom(term_2) and term_2 in @valid_types, do: Atom.to_string(term_2) === term_1
def equal?(term_1, term_2) when is_binary(term_2) and term_2 in @valid_string and is_atom(term_1) and term_1 in @valid_types, do: Atom.to_string(term_1) === term_2
def equal?(term_1, term_2) when is_binary(term_1) and is_binary(term_2) and term_1 in @valid_string and term_2 in @valid_string, do: term_1 === term_2
def equal?(term_1, term_2) when is_atom(term_1) and is_atom(term_2) and term_1 in @valid_types and term_2 in @valid_types, do: term_1 === term_2
def equal?(_, _), do: false
def is_valid?(value, force_atom_version \\ true)
def is_valid?(value, _) when is_atom(value) and value in @valid_types, do: true
def is_valid?(value, false) when is_binary(value) and value in @valid_string, do: true
def is_valid?(_, _), do: false
end
defmodule Checkout.Request do
use Ecto.Schema
import Ecto.Changeset, only: [cast: 3, cast_embed: 3, validate_required: 2, apply_action: 2]
@primary_key false
embedded_schema do
field :key, :string
field :type, Checkout.Request.Type
embeds_one :account, Checkout.Request.Account
embeds_one :owner, Checkout.Request.Owner
end
def create_request(params) do
%__MODULE__{}
|> cast(params, [:key, :type])
|> cast_embed(:account, required: true)
|> cast_embed(:owner, required: true)
|> validate_required([:key, :type])
|> apply_action(:insert)
end
end
defmodule Checkout.Request.Account do
use Ecto.Schema
import Ecto.Changeset, only: [cast: 3, validate_required: 2]
@primary_key false
embedded_schema do
field :account_type, :string # this can be as the top request `type` a custom type that does proper validation
field :account_number, :string
field :branch, :string
end
def changeset(struct, params) do
struct
|> cast(params, [:account_type, :account_number, :branch])
|> validate_required([:account_type, :account_number, :branch])
end
end
defmodule Checkout.Request.Owner do
use Ecto.Schema
import Ecto.Changeset, only: [cast: 3, validate_required: 2]
@primary_key false
embedded_schema do
field :name, :string
end
def changeset(struct, params) do
struct
|> cast(params, [:name])
|> validate_required([:name])
end
end
And then your controller can be:
def create(conn, body) do
case Checkout.Request.create_request(body) do
{:ok, _} -> send_resp(conn, 200, "Request created successfully")
{:error, changeset} -> send_resp(conn, 400, Jason.encode!(%{errors: extract_errors_from_changeset(changeset)}))
end
end
# this should be extracted to its own module, or could be implemented on the Checkout.Request.create_request function itself to output either {:ok, struct} or {:error, already_traversed_map_of_errors}
def extract_errors_from_changeset(%Ecto.Changeset{valid?: false} = changeset) do
Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
Enum.reduce(opts, msg, fn {key, value}, acc ->
String.replace(acc, "%{#{key}}", to_string(value))
end)
end)
end
It also has the benefit that if you validate the params initially at the entry point you can then rely on having a structured map throughout the whole lifecycle of the request and I think that in the end it’s less work that doing add hoc checks on every step. Adding further validations is also quite simple and it also makes testing the controller easier, as there’s only 2 branches, you can then create a test suite for the schema validation and that will be the only thing you need to change when requirements change.
iex(21)> Checkout.Request.create_request(%{key: "44187221816", account: %{account_type: "checking_account", account_number: "00028363-6", branch: "0001"}, owner: %{name: "Eve Montalvão"}, type: "one_type_request"})
{:ok,
%Checkout.Request{
account: %Checkout.Request.Account{
account_number: "00028363-6",
account_type: "checking_account",
branch: "0001"
},
key: "44187221816",
owner: %Checkout.Request.Owner{name: "Eve Montalvão"},
type: :one_type_request
}}
iex(22)> {:error, changeset} = Checkout.Request.create_request(%{key: "44187221816", account: %{account_type: "checking_account", account_number: "00028363-6", branch: "0001"}, owner: %{name: "Eve Montalvão"}})
{:error,
#Ecto.Changeset<
action: :insert,
changes: %{
account: #Ecto.Changeset<
action: :insert,
changes: %{
account_number: "00028363-6",
account_type: "checking_account",
branch: "0001"
},
errors: [],
data: #Checkout.Request.Account<>,
valid?: true
>,
key: "44187221816",
owner: #Ecto.Changeset<
action: :insert,
changes: %{name: "Eve Montalvão"},
errors: [],
data: #Checkout.Request.Owner<>,
valid?: true
>
},
errors: [type: {"can't be blank", [validation: :required]}],
data: #Checkout.Request<>,
valid?: false
>}
> Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} ->
Enum.reduce(opts, msg, fn {key, value}, acc ->
String.replace(acc, "%{#{key}}", to_string(value))
end)
end)
%{type: ["can't be blank"]}
