admin管理员组

文章数量:1433500

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?

Share Improve this question asked Nov 18, 2024 at 13:52 Ian MacIan Mac 336 bronze badges 5
  • 3 I think this is 100% OK to do. It is very clear and easy to understand. Have your coworkers proposed an alternative approach? – Peaceful James Commented Nov 18, 2024 at 19:47
  • Coworkers have proposed passing a list of functions to Enum.find_value/3. So [&coerce_to_number/1, &coerce_to_naive_datetime/1, &coerce_to_string/1] would be passed to Enum.find_value where a case would return {:ok, value} or nil. That just seems like added complexity and obfuscates the purpose of the code in my opinion. – Ian Mac Commented Nov 19, 2024 at 14:34
  • 2 For starters, else 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, how coerce_to_string/1 might have failed. – Aleksei Matiushkin Commented Nov 19, 2024 at 15:52
  • @IanMac The proposed approach from your coworkers is equally clear to me. I think either approach is good – Peaceful James Commented Nov 19, 2024 at 19:49
  • An alternative way, similar in spirit to the Enum.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
Add a comment  | 

1 Answer 1

Reset to default 0

It'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.

本文标签: elixirIs it idiomatic to use the else in with for the happypathStack Overflow