Hey, just some feedback since you asked. I am by no means an expert but this is what I see:
You repeat this pattern a lot in your code:
case get!(@endpoint_url, [], params: %{setcode: set}) do
%HTTPoison.Response{status_code: 200, body: body} ->
Jason.decode(body)
%HTTPoison.Response{status_code: 400, body: body} ->
{:error, Jason.decode!(body)["error"]}
end
So I would recommend defining a helper function somewhere and pipe all your get or get! calls into that. Something like this
defp handle_response({:ok, %HTTPoison.Response{status_code: 200, body: %{"data" => data}}}) do
{:ok, data}
end
defp handle_response({:ok, %HTTPoison.Response{status_code: 200, body: body}}) do
{:ok, body}
end
defp handle_response({:ok, %HTTPoison.Response{status_code: 400, body: body}}) do
{:ok, body["error"]}
end
defp handle_response({:error, error}) do
{:error, error}
end
Allowing you to call it from your function as below. I changed get! to get, not for any reason in particular, only because that is what I had in mind when I wrote the helpers above. I also extracted the Jason.decode call into one of the callbacks provide by HTTPoison.
def process_response_body(body) do
Jason.decode!(body)
end
def get_card_set_information(set) do
get(@endpoint_url, [], params: %{setcode: set})
|> handle_response()
end
Finally, this may be overkill, but I have been reading about macros lately and so I wanted to see if I could dynamically generate all the functions for the API and thought it may be of interest.
In the Ygo module below, I iterate over the list of @endpoint_arities and use unquote fragments to dynamically generate the associated functions. I only tested a couple endpoint and it seemed to work fine. Note in this version some of the expected arguments have changed, i.e. in Ygo.get_card_set_info you have to pass in a map like %{setcode: setcode}.
I’m not saying you should do it this way, but between dynamically generating the functions and adding the handle_response helper, the result is much less code.
defmodule Ygo.API do
use HTTPoison.Base
@base_url "https://db.ygoprodeck.com/api/v7/"
def process_request_url(url) do
@base_url <> url
end
def process_response_body(body) do
Jason.decode!(body)
end
end
defmodule Ygo do
@endpoint_arities [
archetypes: 1,
card_info: 1,
card_sets_info: 1,
card_sets: 0,
random_card: 0
]
for {endpoint, arity} <- @endpoint_arities do
function = "get_#{endpoint}" |> String.to_atom()
case arity do
1 ->
def unquote(function)(params) do
unquote(endpoint)
|> format_endpoint()
|> Ygo.API.get([], params: params)
|> handle_response()
end
0 ->
def unquote(function)() do
unquote(endpoint)
|> format_endpoint()
|> Ygo.API.get()
|> handle_response()
end
end
end
defp format_endpoint(endpoint) do
endpoint
|> Atom.to_string()
|> String.replace("_", "")
|> Kernel.<>(".php")
end
defp handle_response({:ok, %HTTPoison.Response{status_code: 200, body: %{"data" => data}}}) do
{:ok, data}
end
defp handle_response({:ok, %HTTPoison.Response{status_code: 200, body: body}}) do
{:ok, body}
end
defp handle_response({:ok, %HTTPoison.Response{status_code: 400, body: body}}) do
{:ok, body["error"]}
end
defp handle_response({:error, error}) do
{:error, error}
end
end
. It is my first published package, so I’m sure there’s huge room for improvement. Please take a look at the README or the docs and let me know what you think!
