Our codebase contains existing functions to cast values into specific datatypes. I'm trying to leverage these functions to cast an input value of unknown target type into one of three datatypes (number, naive datetime, or string). Our existing functions return an {:ok, new_value}
or {:error, old_value}
tuple. Right now I'm using the following with
logic:
with {:error, value} <- coerce_to_number(value),
{:error, value} <- coerce_to_naive_datetime(value),
{:error, value} <- coerce_to_string(value) do
:error
else
{:ok, value} -> {:ok, value}
end
Personally, I am comfortable with this logic and like how the with
expresses the steps we take when casting the input values. However, I have coworkers who dislike using the else
in the with
for the "happy-path". Is there a more idiomatic way of expressing the logic above?
Our codebase contains existing functions to cast values into specific datatypes. I'm trying to leverage these functions to cast an input value of unknown target type into one of three datatypes (number, naive datetime, or string). Our existing functions return an {:ok, new_value}
or {:error, old_value}
tuple. Right now I'm using the following with
logic:
with {:error, value} <- coerce_to_number(value),
{:error, value} <- coerce_to_naive_datetime(value),
{:error, value} <- coerce_to_string(value) do
:error
else
{:ok, value} -> {:ok, value}
end
Personally, I am comfortable with this logic and like how the with
expresses the steps we take when casting the input values. However, I have coworkers who dislike using the else
in the with
for the "happy-path". Is there a more idiomatic way of expressing the logic above?
1 Answer
Reset to default 0It's fine if your with
statements have an else
clause: it's there for a reason. What is cleaner, however, (as Aleksei has pointed out) is omitting the else
clause when it isn't needed. A red flag for me is any time I have clauses that have the same value on both sides of the ->
, e.g. {:error, error} -> {:error, error}
or {:ok, value} -> {:ok, value}
. That's a hint that maybe you could refactor the code to be more straightforward.
When I have a series of steps or checks that need to apply to a certain value, I tend do structure my statements to look something like this:
with :ok <- first_thing(val),
:ok <- second_thing(val),
:ok <- third_thing(val) do
take_action(val)
end
All my secondary functions return :ok
or {:error, error}
.
In your case, it might make more sense to match on the :error
, but you should make it clear if you are iteratively manipulating the initial value
. In your example, I might guess that your secondary functions are changing the value
, e.g. does coerce_to_number/1
return a modified version of the input value
?
If it IS changing the value, I'd suggest reworking your code because the functions are effectively doing 2 things: they are EITHER coercing (e.g. to a number), OR they are modifying the input value to something else. SRP helps.
If it is NOT changing the value, then why bother returning it? Why not just do :error <- coerce_to_thing(value)
? That way there's no confusion.
Another pattern that you might find useful is the humble ||
operator, e.g. something like this:
converted_value = first_attempt(value) || second_attempt(value) || third_attempt(value) || {:error, "Nothing worked"}
Each secondary function here returns nil
on failure, causing the the ||
to evaluate the thing to its right in the same way that
iex> x = nil || "foo" || "bar"
"foo"
so you can chain together a bunch of functions and take the result from the first one that returns a non-nil value.
Enum.find_value/3
. So[&coerce_to_number/1, &coerce_to_naive_datetime/1, &coerce_to_string/1]
would be passed toEnum.find_value
where acase
would return{:ok, value}
ornil
. That just seems like added complexity and obfuscates the purpose of the code in my opinion. – Ian Mac Commented Nov 19, 2024 at 14:34else
clause above is redundant. Anyway, if this is a made-up contrived example, it seems to hide the actual problem (which is most likely the XY one.) Furthermore, even if this is a real-life example, I struggle to figure out, howcoerce_to_string/1
might have failed. – Aleksei Matiushkin Commented Nov 19, 2024 at 15:52Enum.find_value
but arguably simpler could be:coerce_to_number(v) || coerce_to_naive(v) || coerce_to_string(v) || :error
. – sabiwara Commented Nov 21, 2024 at 23:40