How can I DRY out this code more (Plug.Conn, with statement)

marisradu

Disclaimer: I’m new (still learning) elixir.
Can’t figure how to DRY this code out, I would like to capture the “message” from all blocks and then send the response and halt the connection.

Original code:

def call(conn, _) do
    case some_function(conn) do
      {:ok, response} ->
        put_private(conn, :my_app, %{my_key: response})

      {:error, :error_1 ->
        conn
        |> put_resp_content_type("application/json")
        |> send_resp(403, "Message for error 1")
        |> halt

      {:error, :error_2} ->
        conn
        |> put_resp_content_type("application/json")
        |> send_resp(403, "Message for error 2")
        |> halt

      _ ->
        conn
        |> put_resp_content_type("application/json")
        |> send_resp(403, "Generic message")
        |> halt
    end
  end

I wanted to put everything in a with statement like:

with {:ok, response} <-  some_function(conn) do
     put_private(conn, :my_app, %{my_key: response})
else
     {:error, :error_1} ->
        message = "Message for error 1"
     {:error, :error_2} ->
        message = "Message for error 2"
      _ ->
        message = "Generic message"
    end

I’m pretty sure there is a better way to capture the “message” and also not sure where to put the “response” as inside the “else” clause will need to put it for all clauses, outside will also be triggered if there is no error.

conn
|> put_resp_content_type("application/json")
|> send_resp(403, message)
|> halt

Any hint or constructive criticism in case you think I’m going in the wrong direction will be appreciated.
Thanks.

9 comments

#plug #code-review #dry

0 1563 9

2018-05-11 14:04:18 UTC

Showing Posts 1 to 9

Show Best Posts
Show All (oldest first)
Show All (newest first)

LostKobrakai

Using with was already a good idea, but sometimes just using one flow control type is just not enough:

def call(conn, _) do
	with {:ok, response} <- some_function(conn) do
		put_private(conn, :my_app, %{my_key: response})
	else
		err -> 
			message =
			  case err do
					{:error, :error_1} -> "Message for error 1"
					{:error, :error_2} -> "Message for error 2"
					_ -> "Generic message"
				end

			conn
			|> put_resp_content_type("application/json")
			|> send_resp(403, message)
			|> halt
	end
end

Post #1

hubertlepicki

Generally if you also do format the code (buttons above or with indentation) properly you get better chance of people responding - it’s quite difficult to decipher not formatted code and programmers are obviously lazy

Post #2

marisradu

Damn … did do that, but missed to check in the preview pane.
Definitely going to get a strong coffee now.

Thanks for the notification.

Post #3

mbuhot

When in doubt, use more functions

def call(conn, _) do
  with {:ok, response} <- some_function(conn) do
    put_private(conn, :my_app, %{my_key: response})
  else
    err -> 
      message = error_message(err)
      conn
      |> put_resp_content_type("application/json")
      |> send_resp(403, message)
      |> halt
  end
end
    
defp error_message({:error, :error_1}), do: "Message for error 1"
defp error_message({:error, :error_2}), do: "Message for error 2"
defp error_message(_), do: "Generic message"

Post #4

idi527

I don’t see much reason to use with when a flat case would be enough (as in your case).

Post #5

Nicd

Yes, iterating on the last response:

def call(conn, _) do
  case some_function(conn) do
    {:ok, response} ->
      put_private(conn, :my_app, %{my_key: response})

    err -> 
      message = error_message(err)
      conn
      |> put_resp_content_type("application/json")
      |> send_resp(403, message)
      |> halt
  end
end
    
defp error_message({:error, :error_1}), do: "Message for error 1"
defp error_message({:error, :error_2}), do: "Message for error 2"
defp error_message(_), do: "Generic message"

Post #6

LostKobrakai

I’m mostly of the same opinion, but with might communicate a bit more the monad-like success/error railway-ing while case is a bit more “equal cases” in it’s semantics. Especially in plugs where one branch does halt the pipeline I see myself more drawn to using with.

Post #7

peerreynders

I sense a lack of commitment

def call(conn, _) do
  with {:ok, response} <- some_function(conn) do
    put_private(conn, :my_app, %{my_key: response})
  else
    err -> 
      call_error(err, conn)
  end
end
    
defp call_error({:error, :error_1}, conn), do: forbidden_response(conn, "Message for error 1")
defp call_error({:error, :error_2}, conn), do: forbidden_response(conn, "Message for error 2")
defp call_error(_, conn), do: forbidden_response(conn)

defp forbidden_response(conn, msg \\ "Generic Message"), do: error_response(conn, 403, msg)

defp error_response(conn, status, msg) do
  conn
  |> put_resp_content_type("application/json")
  |> send_resp(status, msg)
  |> halt
end

Post #8