Pattern Matching Functions to solve "Complex Branching"

[dotdotdotpaul]

Ref. https://github.com/lucasvegi/Elixir-Code-Smells#complex-branching

I'm not a fan of having a gazillion private functions named with overly "descriptive" names, like is described under "Complex Branching" (see above).

I absolutely agree you don't want large case or cond (or if/else for that matter) blocks where you have to trace indents across screens of code. But I wonder if the better refactoring pattern isn't moving those blocks into individual functions, but rather, using pattern matching functions to replace the conditional altogether.

Instead of the suggested

def get_customer(customer_id) when is_integer(customer_id) do
  case get("/customers/#{customer_id}") do
    {:ok, %Tesla.Env{status: 200, body: body}} -> success_api_response(body)
    {:ok, %Tesla.Env{body: body}} -> x_error_api_response(body)
    {:error, _} = other -> y_error_api_response(other)
  end
end

defp success_api_response...
defp x_error_api_response...
defp y_error_api_response...

Would this be better handled by piping to a handler function that pattern matches on the result to handle "branching"?

def get_customer(customer_id) when is_integer(customer_id) do
  get("/customers/#{customer_id}")
  |> handle_api_response()
end

defp handle_api_response({:ok, %Tesla.Env{status: 200, body: body}}) do
  # do stuff
end
defp handle_api_response({:ok, %Tesla.Env{body: body}} do
  # do other stuff
end
defp handle_api_response({:error, _}) do
  # handle error
end

This has the benefit of keeping each branch in separate functions, without cluttering function namespace. Because the function names are the same, linters will flag if the handler functions get "separated" by new code (I would argue that it is important to keep these handlers together, regardless of which pattern is used -- but if the functions are named differently, there's nothing forcing that proximity).

It's probably hard to make a generalization about which is better (with the small one-liner function bodies from the original example, it's hard to say that any refactoring is worthwhile at all).

Just thought I'd start the conversation. 😄

[lucasvegi]

Thanks for raising this discussion @dotdotdotpaul !
Your refactoring suggestion is identical to the first refactoring I proposed in the catalog. It was modified in this commit 2c7c1a1

This modification was motivated by a good discussion I had with @josevalim on this issue(#4). I think it's worth taking a look at it. Anyway, we can keep this discussion open to thinking about it together.

[dotdotdotpaul]

Heh, go figure! At least now we know Discussions are working. 😁

[lucasvegi]

Exactly!