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

Chat / Discussions
1

First off, I have this “working” with happy_path (It seems to execute properly, but it’s difficult to exercise the error paths). I am creating this topic for a couple main reasons:

  1. Is there a “better” way?
  2. Why isn’t my with statement compiling?
  3. Is there a way with with statements to have multi-line clauses.
  4. Are nested with statements allowed/possible?

So I’ll come back to these questions after the code:

Pyramid of doom with branching case statement in the middle

def start_or_resume_session(session_id) when is_bitstring(session_id) do
  case IbGib.Expression.Supervisor.start_expression({"session", "gib"}) do
    {:ok, root_session} ->
      case get_session_ib(session_id) do
        {:ok, session_ib} ->
          case get_latest_session_ib_gib(session_id, root_session) do
            # I want to do stuff if it's nil
            {:ok, nil} ->
              case (root_session |> instance(session_ib)) do
                {:ok, {_, session}} ->
                  case session |> get_info do
                    {:ok, session_info} ->
                      case get_ib_gib(session_info) do
                        {:ok, session_ib_gib} -> {:ok, session_ib_gib}
                        {:error, reason} -> {:error, reason}
                      end
                    {:error, reason} -> {:error, reason}
                  end
                {:error, reason} -> {:error, reason}
              end

            # Return it if not nil
            {:ok, existing_session_ib_gib} -> {:ok, existing_session_ib_gib}

            {:error, reason} -> {:error, reason}
          end
        {:error, reason} -> {:error, reason}
      end
    {:error, reason} -> {:error, reason}
  end
end

Attempt with nesting with (doesn’t compile)

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, session_ib_gib} <-
      case get_latest_session_ib_gib(session_id, root_session) do
        {:ok, nil} ->
          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

        {:ok, existing_session_ib_gib} -> {:ok, existing_session_ib_gib}

        {:error, reason} -> {:error, reason}
      end,
      do: {:ok, session_ib_gib}
  end
end

Attempt with nesting happy_path (works, I think)

def start_or_resume_session(session_id) when is_bitstring(session_id) do
  happy_path do
    {:ok, root_session} = IbGib.Expression.Supervisor.start_expression({"session", "gib"})
    {:ok, session_ib} = get_session_ib(session_id)
    {:ok, existing_session_ib_gib} = get_latest_session_ib_gib(session_id, root_session)
    {:ok, session_ib_gib} =
      if (existing_session_ib_gib == nil) do
        Logger.debug "nil case...are we still on happy path?"
        # it's nil, so create a new one and return that
        # Are we still on the happy_path?  No. So nest another path?
        happy_path do
          {:ok, {_, session}} = root_session |> instance(session_ib)
          {:ok, session_info} = session |> get_info
          {:ok, session_ib_gib} = get_ib_gib(session_info)
          {:ok, session_ib_gib}
        end
      else
        {:ok, existing_session_ib_gib}
      end
    {:ok, session_ib_gib}
  end
end

Questions

Is there a “better” way?

Overall, could/should I refactor something out? I’m not a huge fan of do_x_or_y functions, but this seems pretty straight-forward to me, and the long-ish name is more of being precise.

Or, is there a more concise/appropriate way of nesting happy_path statements?

Why isn’t my with statement compiling?

The error changes, depending on which permutation of , do:, or do end block in the with statement I use. It’s either there is an unexpected end later on in the file, or that there is an illegal character before a comma (,) on the with block. I think it’s balking on the case statement line. Are case statements, or rather full do..end blocks not allowed with with clauses?

Is there a way with with statements to have multi-line clauses.

This is probably related to the previous question.

Everything I’ve seen/read on with has a single line clause, with each line ending with a comma before the do block. Is there a way to have a multi-line clause?

Are nested with statements allowed/possible?

I can’t seem to figure out how to nest with statements, and I just think that I should. I’m still a little fuzzy on some of the , do: vs do...end blocks thing (I didn’t come from a Rails background).

4 Likes
2

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
6 Likes
3

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. :smile:

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?

1 Like
4

@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 :smiley:

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
4 Likes
5

Looks really clean to me :slight_smile:
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

3 Likes
6

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.

4 Likes
7

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?

1 Like
8

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).

1 Like
9

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

1 Like