Is this a violation of the anti-pattern Complex Else Clauses in With"?

No to me you are already following the recommendations, the return codes are specific and not overlapping.

Great and ty. That also how I read the anti-pattern docs. But glad to have double checked.

It seems borderline to me. Even though the errors are pretty easy to map back to the different with clauses, it is still extra “strain” rather than handling the different types of errors in the caller.

Would this be better?

defmodule BookClubWeb.APIController do
  use BookClubWeb, :controller
  alias BookClub.Books

  def books(conn, %{"title" => title}) do
    books = Books.list_books_with_active_or_first_page(filter: title)
    render_pretty_json(conn, books)
  end

  def books(conn, _params) do
    books = Books.list_books_with_active_or_first_page()
    render_pretty_json(conn, books)
  end

  def book(conn, %{"id" => id}) do
    with {:ok, id} <- parse_id(id, conn),
         {:ok, book} <- fetch_book(id, conn) do
      render_pretty_json(conn, book)
    end
  end

  defp render_pretty_json(conn, data) do
    json_string = Jason.encode!(data, pretty: true)
    conn
    |> put_resp_content_type("application/json")
    |> send_resp(200, json_string)
  end

  defp parse_id(id, conn) do
    case Integer.parse(id) do
      {id, _} when id >= 1 ->
        {:ok, id}

      _ ->
        conn
        |> put_status(:bad_request)
        |> json(%{error: "Invalid ID"})
    end
  end

  defp fetch_book(id, conn) do
    case Books.get_book_with_active_or_first_page(id) do
      nil ->
        conn
        |> put_status(:not_found)
        |> json(%{error: "Book not found"})

      book ->
        {:ok, book}
    end
  end
end

Sorry on phone so can’t type out code, but I think in your case I would take the conn stuff out of the parse and fetch functions. I would have the book function return either ok or error tuple and then do the conn stuff on the return from that function. That way you are talking away the need to ever care about which error comes from which with clause. You just know the book function can return different kinds of errors

I think this solution is harder to follow. I do not like to see a function called “parse_id” modifying conn. Explicit error case clauses in the first code sample seems much clearer.

In this case, I’d have the helper functions return a 3-tuple in the error case, e.g. {:error, status_code, message} and a single else block to update the conn

Maybe it’s copy pasta, but that last function clause def book(conn, %{"id" => id}) do won’t match as there’s a “catch all” def books(conn, _params) do right before it. Order matters here since Elixir tries each clause until it finds one that matches so that “catch all” should come last.

book != books

Ahh, good catch – mea culpa!