Hacker News Reader

Comment by pg_bot | Hacker News Reader

The following code could be written much better by using the cond operator.

    with {:is_email, true} <- {:is_email, is_email_address?(email)},
         {:is_available, true} <- {:is_available, EmailAddresses.is_available(email)} do
      ...
    else
      {:is_email, false} ->
        {:error, :bad_request}

      {:is_available, false} ->
        {:error, :conflict}
    end

    cond do 
      !email_address?(email) -> {:error, :bad_request}
      !EmailAddresses.available?(email) -> {:error, :conflict}
      true -> {:ok, email}
    end

This gets rid of unnecessary duplication, and I think is easier to understand.

View on HN

Keathley did a good job discussing this in https://keathley.io/blog/good-and-bad-elixir.html#:~:text=Av.... The preferred style is to specify the errors in separate functions e.g.

    def main do
      with {:ok, response} <- call_service(data),
           {:ok, decoded}  <- decode(response),
           {:ok, result}   <- store_in_db(decoded) do
        :ok
      end
    end

where call_service, decode and store_in_db return the specific errors like {:error, :bad_request}, {:error, :conflict}.

Isn't there a way of piping the ok results through? e.g.

  data
  |> call_service
  |> an_ok_unwrapper(decode)
  |> an_ok_unwrapper(store_in_db)

Or is `with` the way of doing this for ok/error results?

`with` is more widely used, i think, but there's also `Kernel.then/2`:

    data
    |> call_service
    |> then(fn {:ok, decode} -> decode_file(decode) end)
    |> then(fn {:ok, file} -> store_file(file) end)

your solution also works if you define the function headers with a pattern match against the tuple, but then you have this extra function hanging around. Feels like a style thing more than anything else.

This approach is not equivalent since it uses a strict match in the function head inside `then`. It will raise a `FunctionClauseError` if a value not matching `{:ok, _}` is passed in.

I mean, yes, you're right that the failure mode of the `then/2` approach works differently than `with`. I think most semi-experienced elixir devs would recognize that the function used with `then/2` needs to have a pattern that matches expected returns—as does `with/else` if you want to be able to continue on the happy path of your program.

For those that don't realize that, caveat lector.

If that's the behavior desired, experienced Elixir devs would use the match operator since it's more conventional, requires fewer characters, and eliminates multiple unnecessary function calls. The thread was about monadic transformation operations similar to `and_then` on Result and Option in Rust or Promise#then in JavaScript.

Doing it with `with` is definitely the community standard. There are attempts like https://github.com/vic/ok_jose to make it more like "do" notation.

Thank you for the reference (not finished it yet).

Worth mentioning functions that can error, should follow the {:ok, response} | {:error, reason} pattern. Because if such a function returns response | {:error, reason}, then if we are inside a with clause and we want to capture the response and use it in the next with clause, such capture value can be either response or {:error, reason} - which goes around the pattern matching.

  with response_f1 <- f1(),
       {:ok, response_f2} <- f2(response_f1) do
    # do something
  else
    {:error, reason_f1} ->
      # we will never come here
      # because the returned value from f1
      # is already matched to the variable response_f1 
  end

I've seen that recommendation often but I still dislike it, since it falls apart whenever you need to do the same logic on multiple pieces of data. It only makes sense if every single with-clause is doing something hugely different.

A simple example:

    with {:ok, cleaned_first}  <- sanitize(data.first),
         {:ok, cleaned_middle} <- sanitize(data.middle),
         {:ok, cleaned_last}   <- sanitize(data.last), 
         {:ok, final}          <- combine(first, middle, last) do:
      {:ok, "Name is" <> final}
    else
      {:error, :illegal_character} ->
        {:error, :illegal_first_name_or_middle_name_or_last_name}
      other ->
        other
    end

The above sucks because it's not clear which data was bad or which step failed.

So let's talk about the alternatives...

__________________

First, the "Lots Of Methods" approach, where you just add as many different private methods as you need, where each one works almost the same except for a distinctly different error:

    {:ok, cleaned_first}  <- sanitize_first(data.first),
    {:ok, cleaned_middle} <- sanitize_middle(data.first),
    {:ok, cleaned_last}   <- sanitize_last(data.first),

I feel that sucks because it's tedious, error-prone boilerplate, even if they're just wrappers around a sanitize/1 that does the real work. Also any new error-atoms being introduced are scattered down the file rather than near where you might want to match/test them.

__________________

Second, the "Augment One Method With Causal Data" version:

    {:ok, cleaned_first}  <- sanitize(data.first, :phase_first),
    # ... else ...
    {:error, :illegal_character, :phase_first} -> {:error, :illegal_first_name}

This is a marginal improvement, but we're still contaminating methods like "sanitize" with junk they don't actually need to know in order to do so their job, passing a piece of opaque data down and up the stack unnecessarily.

__________________

Third, what if we carefully isolate the concerns/complexity to the with-statement... Hey! We're right back to where we started!

The "Augment The With Clauses" approach, which I argue is least-bad:

    {_phase, {:ok, cleaned_first}}   <- {:phase_first,  sanitize(data.first)},
    {_phase, {:ok, cleaned_middle}}  <- {:phase_middle, sanitize(data.middle)},
    {_phase, {:ok, cleaned_last}}    <- {:phase_last,   sanitize(data.last)},
    # ... else ...
    {:phase_first,  {:error, :illegal_character}} -> {:error, :illegal_first_name}
    {:phase_middle, {:error, :illegal_character}} -> {:error, :illegal_middle_name}
    {:phase_last,   {:error, :illegal_character}} -> {:error, :illegal_last_name}

i agree that with statements aren't the easiest to understand, especially for a beginner, but I think the value in having the entirety of the happy path in the initial block is very helpful for understanding the flow of the feature, once you grok the syntax.

Reply on news.ycombinator.com