with .. do .. else .. end #4085

Closed
benwilson512 opened this Issue Dec 18, 2015 · 12 comments

Projects

None yet

7 participants

@benwilson512
Contributor

I want to look at the viability of allowing an else clause on the newly introduced with. Right now with obviously flattens the success cases of matching, but it also effectively flattens the error cases. Unfortunately there isn't a built in mechanism with with to take advantage of the flattened error cases. So for example a common pattern that has arisen in my code around building dependent ecto models is:

  def create(params) do
    with {:ok, organization} <- create_organization(params),
         {:ok, address}      <- create_address(organization, params) do
      {:ok, %{organization | address: address}}
    end
    |> case do
      {:ok, organization} -> {:ok, organization}
      {:error, changeset} -> {:error, changeset |> error_string}
    end
  end

This could more succinctly written if with had an else clause

  def create(params) do
    with {:ok, organization} <- create_organization(params),
         {:ok, address}      <- create_address(organization, params) do
      {:ok, %{organization | address: address}}
    else
      {:error, changeset} -> {:error, changeset |> error_string}
    end
  end

Thoughts?

@lexmag
Member
lexmag commented Dec 19, 2015

I've had this discussion with @fishcakez on IRC, as I remember we agreed that it'd be nice to have else which works in a similar way to else in try. (James, please correct me if I'm wrong.)

@alco
Member
alco commented Dec 19, 2015

@lexmag Could you clarify? else in try is evaluated if the expression did not fail. According to @benwilson512's proposal, else in with should be invoked if a match fails. Otherwise it won't add anything beyond what can be achieved by piping with into case do.

@lexmag
Member
lexmag commented Dec 19, 2015

@alco I mean else in with should be similar to else in try in terms of supporting patterns and not to be just any expression like else in if, invocation of course is different.
In other words, it is what is proposed here.

@benwilson512
Contributor

I'm glad you brought up the use case of else in try as I was worried that using else with matches lacked precedent.

I suppose the remaining question is what to do if all of the else clauses fail. MatchError ? That seems the right choice to me. If you use with without else you match on success cases and return any error cases. If you use with as well as else you're exhaustively enumerating both success and failure patterns, or running into a MatchError just like case

@alco
Member
alco commented Dec 20, 2015

try raises a TryClauseError.

A small correction to @benwilson512's comment:

If you use with as well as else you're exhaustively enumerating both success and failure patterns, [...]

You're not enumerating successes, only failures. In the else part at least. I now realize you probably meant the combination of the initial matches and the else-clauses.

I think your proposal can be described in simpler terms:

Using with, one can write a sequence of matches and if all of them succeed, the body between do and end will only be evaluated. If a match fails, no further expressions are evaluated and the non-matched value is returned. If there is an else clause, the non-matched value will be passed to it where it will be matched against a list of clauses like in a case. This allows us to adjust what is being returned from with in the case of a failed match.

@almassapargali

Totally would love to see this in Elixir, makes a lot of sense, and already come up with some cases I'd use this.

@almassapargali

Not going further, if @benwilson512 used Repo.transaction, his code could be:

Repo.transaction fn ->
  with {:ok, organization} <- create_organization(params),
       {:ok, address}      <- create_address(organization, params) do
    %{organization | address: address}
  else
    {:error, changeset} -> Repo.rollback(changeset)
  end
end

Of course we can do this with |> case do, but I think this one is much cleaner.

@alco
Member
alco commented Dec 24, 2015

;)

Repo.transact_with {:ok, organization} <- create_organization(params),
                   {:ok, address}      <- create_address(organization, params)
do
  %{organization | address: address}
else
  # This point is reached after the transaction rolls back
  {:error, error} -> :sorry
end

offtopic: I wish Elixir supported line breaking with \ or similar, to be able to write code like this:

Repo.transact_with \
  {:ok, organization} <- create_organization(params),
  {:ok, address}      <- create_address(organization, params)
do
...
end

Function-name-length-independent indentation helps a lot when reading the code IMO.

@josevalim
Member

offtopic: I wish Elixir supported line breaking with \ or similar, to be able to write code like this:

It does. :)

@alco
Member
alco commented Dec 24, 2015

Awesome! Somehow I thought it wasn't supported when I tried it a few days ago.

@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Dec 25, 2015
@liveforeverx liveforeverx Optional else clauses can be added to special form with
Reference: #4085
c1968ac
@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Dec 25, 2015
@liveforeverx liveforeverx Optional else clauses can be added to special form with
Reference: #4085
26b4a4f
@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Dec 27, 2015
@liveforeverx liveforeverx Optional else clauses can be added to special form with
Reference: #4085
152f299
@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Dec 28, 2015
@liveforeverx liveforeverx Optional else clauses can be added to special form with
If else conditions are given, then wrapping case will be generated
around with case clauses. In a case, if else conditions doesn't have
a match all clause, than it will be added clause, which raise
WithClauseError.

Reference: #4085
95e99cb
@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Dec 28, 2015
@liveforeverx liveforeverx Optional else clauses can be added to special form with
If else conditions are given, then wrapping case will be generated
around with case clauses. In a case, if else conditions doesn't have
a match all clause, than it will be added clause, which raise
WithClauseError.

Reference: #4085
323b97f
@lexmag lexmag added this to the v1.3.0 milestone Dec 29, 2015
@liveforeverx liveforeverx added a commit to liveforeverx/elixir that referenced this issue Jan 1, 2016
@liveforeverx liveforeverx Optional else clauses can be added to special form with
If else conditions are given, then wrapping case will be generated
around with case clauses. In a case, if else conditions doesn't have
a match all clause, than it will be added clause, which raise
WithClauseError.

Reference: #4085
0aac5a7
@vic
Contributor
vic commented Jan 20, 2016

Just for reference, I created a tiny happy macro that just converts code into a chain of cases, I'm currently using it in a couple of projects of mine, as IMHO it's cleaner than with's syntax. It also allows an else clause for handling non-matching values. The downside is variables would leak as it's just a plain old case.

@liveforeverx
Contributor

@vic For variables not leaking in your implementation, you can transform expression in happy_path macro to with expression instead of cases.

I personally find with cleaner as happy_path, because it is much more explicit in a case, where to define cases and where not:

with {:ok, width} <- Map.fetch(opts, :width),
     double_width = width * 2,

And, after using it our codebase, later separation of do I find better, because of better separation of concerns. And most use cases, where I used were like:

with .... <- get_data,
     .... <- another_get_or_validating
do
   pure_transformation
end

where happy_path will be

happy_path do
 .... = get_data
 .... = another_get_or_validating
 pure_transformation
end

But it is really, my IMHO.

@lexmag lexmag closed this Feb 6, 2016
@OnorioCatenacci OnorioCatenacci added a commit to OnorioCatenacci/elixir that referenced this issue Mar 10, 2016
@liveforeverx @OnorioCatenacci liveforeverx + OnorioCatenacci Optional else clauses can be added to special form with
If else conditions are given, then wrapping case will be generated
around with case clauses. In a case, if else conditions doesn't have
a match all clause, than it will be added clause, which raise
WithClauseError.

Reference: #4085
6bee3ec
@OnorioCatenacci OnorioCatenacci added a commit to OnorioCatenacci/elixir that referenced this issue Mar 15, 2016
@liveforeverx @OnorioCatenacci liveforeverx + OnorioCatenacci Optional else clauses can be added to special form with
If else conditions are given, then wrapping case will be generated
around with case clauses. In a case, if else conditions doesn't have
a match all clause, than it will be added clause, which raise
WithClauseError.

Reference: #4085
0fa97fa
@vic vic referenced this issue in lasseebert/kase Jun 20, 2016
Closed

Add happy to alternatives. #9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment