CODEWITHSUNDEEP
pattern-matchingelixir
CallMeNorm
CallMeNorm

Reputation: 2605

Pattern matching return value maps in elixir

At work I ran into a bit of code that looked inelegant and the compiler complained that we had unused variables. I was wondering what the idiomatic way of handling this case is.

defp parse_create_case_response({create_status, http_status, body}) do
    case {create_status, http_status} do
        {:ok, 201} ->
            %{"errors" => errors, "id" => id, "success" => success} = Poison.decode! body
        _other ->
            {:error, %{message: "Internal error", error_code: :internal_server_error}}
    end
end

The compiler complained that errors, id, and error were all unused. I understand why they are unused, but I'm wondering how I should handle this. Should I put a _ in front of each variable to tell the compiler that they are unused? Or do something else entirely?

There are other problems with the code that will be addressed in code review, I'm just wondering how I should help my colleague get past the linter.

Upvotes: 1

Views: 833

Answers (1)

Harrison Lucas
Harrison Lucas

Reputation: 2961

From the looks of it, I assume you are matching the result of this function two to a variable in another function. The compiler is telling you that they are being unused and should be a good hint that maybe the approach at a whole could be refactored. Without seeing the calling code it's hard, but I'd maybe move the matching to the calling function and just returned the parsed response body.

This will make the linter happy :)

So this:

defp parse_create_case_response({create_status, http_status, body}) do
    case {create_status, http_status} do
        {:ok, 201} ->
            %{"errors" => errors, "id" => id, "success" => success} = Poison.decode! body
        _other ->
            {:error, %{message: "Internal error", error_code: :internal_server_error}}
    end
end

Will have the same effect as this:

  defp parse_create_case_response({{:ok}, {201}, _body}) do
    Poison.decode! body
  end

  defp parse_create_case_response(_response) do
    {:error, %{message: "Internal error", error_code: :internal_server_error}}
  end

The reason we don't need to pattern match on the second one is because if it fails to match the first function head then we really don't care what the result is as we know what we have to return.

HOWEVER If you are adamant on returning the map then you could change it to to:

  defp parse_create_case_response({{:ok, {201}, body}) do
    body
    |> Poison.decode!
    |> generate_map
  end

  defp parse_create_case_response(_response) do
    {:error, %{message: "Internal error", error_code: :internal_server_error}}
  end


  defp generate_map(%{"errors" => errors, "id" => id, "success" => success}) do 
    %{"errors" => errors, "id" => id, "success" => success}
  end

This isn't as pretty but it's still pretty clear what's going on. Again without knowing exactly what the context of this method is, it's hard to do any better.

Upvotes: 3

Related Questions