Case pyramid of doom, nested `with`, nested `happy_path`

Could do something like

def start_or_resume_session(session_id) when is_bitstring(session_id) do
  with {:ok, root_session} <- IbGib.Expression.Supervisor.start_expression({"session", "gib"}),
       {:ok, session_ib} <- get_session_ib(session_id),
       {:ok, existing_session_ib_gib} <- latest(session_id, root_session) do
    {:ok, existing_session_ib_gib}
  else
    {:error, reason} ->  {:error, reason}
  end
end

def latest(session_id, root_session) do
  get_latest_session_ib_gib(session_id, root_session)
  |>handle_latest_session_ib_gib(root_session,session_ib)
end

def handle_latest_session_ib_gib({:ok, nil},root_session,session_ib) do
  with  {:ok, {_, session}} <- instance(root_session, session_ib),
        {:ok, session_info} <- get_info(session),
        {:ok, session_ib_gib} <- get_ib_gib(session_info) do
    {:ok, session_ib_gib}
  else
    {:error, reason} ->  {:error, reason}
  end
end

def handle_latest_session_ib_gib(res,_,_), do: res

It looks like you had the gut feeling of refactoring out into another function (or two as in this case)…which, I just have to say that it compiled with only one slight tweak (including session_ib in the latest function signature): That is pretty darn impressive considering the esoteric domain language.

So this refactoring or a similar one would be how to make it better, and that is kind of what I also am leaning towards. Perhaps this is an indication that a nested with or happy_path statement would be in general a code smell? In which case, it would make the other questions moot, and condensing any possible multi-line statements into single-line functions could indicate a cleaner overall design?

@andre1sk Here is what I’ve come up with based on your refactoring. The key I think is moving the case statement pattern matching to function pattern matching as you showed in your response. It definitely looks a lot cleaner

Thanks a lot for your help!

  def start_or_resume_session(session_id) when is_bitstring(session_id) do
    with {:ok, root_session} <- IbGib.Expression.Supervisor.start_expression({"session", "gib"}),
      {:ok, session_ib} <- get_session_ib(session_id),
      {:ok, latest} <- get_latest_session_ib_gib(session_id, root_session),
      {:ok, session_ib_gib} <- create_session_if_needed(latest, root_session, session_ib) do
      {:ok, session_ib_gib}
    else
      {:error, reason} -> {:error, reason}
    end
  end
  def start_or_resume_session(unknown_arg) do
    {:error, emsg_invalid_arg(unknown_arg)}
  end

  defp create_session_if_needed(existing, root_session, session_ib)
    when is_nil(existing) do
    with {:ok, {_, session}} <- root_session |> instance(session_ib),
      {:ok, session_info} <- session |> get_info,
      {:ok, session_ib_gib} <- get_ib_gib(session_info) do
      {:ok, session_ib_gib}
    else
      {:error, reason} -> {:error, reason}
    end
  end
  defp create_session_if_needed(existing, _, _) do
    {:ok, existing}
  end

Looks really clean to me
In Elixir composing small easy to read functions is the favored approach, so having deep nesting control flow structures would prob be considered an anti-pattern

Btw, the reason why your first example using with failed is because you had do blocks inside do blocks which caused a precedence issue. The suggested refactorings are definitely the way to go though, there is no reason to have such a large function as the one initially posted. Breaking the code into smaller functions will be helpful regardless if you are using case, with or happy_path.

Thanks for this comment! I’m still scratching my head though, as I don’t come from a Ruby background and some of the nuances of blocks still evade me. In the above example, do you mean the implicit do before the case statement?

I’ve made a simplified snippet module:

defmodule DoBlocks do
  def foo do
    with :ok <- bar(),
      :ok <- case baz do
          :ok -> :ok
          :error -> :error
        # end, do: IO.puts "ok yo" # doesn't compile
        end do
      IO.puts "ok yo" # doesn't compile either
    end
  end

  def bar, do: IO.puts "bar"
  def baz, do: :ok
end

This particular example is relatively complicated since it includes both a with and a case. Could you give me a simplified nested do problem without the with/case statement in code for me? Also, is this related to why with clauses have only one line?

I use happy_path (excessively in phoenix…) but anytime I would ever think of having a nested one is when I would break that out into a function and happy_path in it with the head of the function matching on what is needed (with a default path to send an error to the parent happy_path to send to the webpage instead of 500’ing).

Thanks for your input. It sounds like everyone is agreed that a nested with/happy_path is a code smell: