From 0abf8af3f95b341fa5e04ff4c93e7259e322de7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lui=CC=81s=20Arteiro?= Date: Thu, 12 Jan 2023 18:47:35 +0000 Subject: [PATCH] fix: Removing try/rescue. #256 --- api.md | 102 ++++++++++++------ lib/app/item.ex | 21 +++- lib/app/timer.ex | 17 ++- .../controllers/api/item_controller.ex | 34 +++--- .../controllers/api/timer_controller.ex | 34 +++--- 5 files changed, 143 insertions(+), 65 deletions(-) diff --git a/api.md b/api.md index 188c8c39..2d1b003f 100644 --- a/api.md +++ b/api.md @@ -310,25 +310,29 @@ defmodule AppWeb.API.ItemController do alias App.Item import Ecto.Changeset - def show(conn, params) do - id = Map.get(params, "id") - - try do - item = Item.get_item!(id) - json(conn, item) - rescue - Ecto.NoResultsError -> - errors = %{ - code: 404, - message: "No item found with the given \'id\'.", - } - json(conn |> put_status(404), errors) + def show(conn, %{"id" => id} = _params) do + case Integer.parse(id) do + # ID is an integer + {id, _float} -> + case Item.get_item(id) do + nil -> + errors = %{ + code: 404, + message: "No item found with the given \'id\'." + } + json(conn |> put_status(404), errors) + + timer -> + json(conn, timer) + end - Ecto.Query.CastError -> + # ID is not an integer + :error -> errors = %{ code: 400, - message: "The \'id\' is not an integer.", + message: "The \'id\' is not an integer." } + json(conn |> put_status(400), errors) end end @@ -460,25 +464,29 @@ defmodule AppWeb.API.TimerController do json(conn, timers) end - def show(conn, params) do - id = Map.get(params, "id") - - try do - timer = Timer.get_timer!(id) - json(conn, timer) - rescue - Ecto.NoResultsError -> - errors = %{ - code: 404, - message: "No timer found with the given \'id\'.", - } - json(conn |> put_status(404), errors) + def show(conn, %{"id" => id} = _params) do + case Integer.parse(id) do + # ID is an integer + {id, _float} -> + case Timer.get_timer(id) do + nil -> + errors = %{ + code: 404, + message: "No timer found with the given \'id\'." + } + json(conn |> put_status(404), errors) + + timer -> + json(conn, timer) + end - Ecto.Query.CastError -> + # ID is not an integer + :error -> errors = %{ code: 400, - message: "The \'id\' is not an integer.", + message: "The \'id\' is not an integer." } + json(conn |> put_status(400), errors) end end @@ -620,7 +628,7 @@ and serialize them as `JSON` objects so they can be returned to the person using the API! ✨ -## 4. Listing `timers` and validating updates +## 4. Listing `timers` and `items` and validating updates Let's implement `list_timers/1` in `lib/app/timer.ex`. @@ -638,6 +646,38 @@ Simple, isn't it? We are just retrieving every `timer` object of a given `item.id`. +We are also using `Item.get_item/1` +and `Timer.get_timer/1`, +instead of using +[bang (!) functions](https://stackoverflow.com/questions/33324302/what-are-elixir-bang-functions). +We are not using bang functions +because they throw Exceptions. +And using `try/rescue` constructs +[isn't a good practice.](https://elixir-lang.org/getting-started/try-catch-and-rescue.html) + +To validate parameters and return errors, +we need to be able to "catch" these scenarios. +Therefore, we create non-bang functions +that don't raise exceptions. + +In `app/lib/timer.ex`, +add `get_timer/1`. + +```elixir + def get_timer(id), do: Repo.get(Timer, id) +``` + +In `app/lib/item.ex`, +add `get_item/1`. + +```elixir + def get_item(id) do + Item + |> Repo.get(id) + |> Repo.preload(tags: from(t in Tag, order_by: t.text)) + end +``` + Digressing, when updating or creating a `timer`, we want to make sure the `stop` field diff --git a/lib/app/item.ex b/lib/app/item.ex index 71d6cb7b..3b3a22b8 100644 --- a/lib/app/item.ex +++ b/lib/app/item.ex @@ -55,7 +55,7 @@ defmodule App.Item do end @doc """ - Gets a single item. + `get_item!/1` gets a single Item. Raises `Ecto.NoResultsError` if the Item does not exist. @@ -74,6 +74,25 @@ defmodule App.Item do |> Repo.preload(tags: from(t in Tag, order_by: t.text)) end + @doc """ + `get_item/1` gets a single Item. + + Returns nil if the Item does not exist + + ## Examples + + iex> get_item(1) + %Timer{} + + iex> get_item(1313) + nil + """ + def get_item(id) do + Item + |> Repo.get(id) + |> Repo.preload(tags: from(t in Tag, order_by: t.text)) + end + @doc """ Returns the list of items where the status is different to "deleted" diff --git a/lib/app/timer.ex b/lib/app/timer.ex index bd36760e..bbd7a95b 100644 --- a/lib/app/timer.ex +++ b/lib/app/timer.ex @@ -42,7 +42,7 @@ defmodule App.Timer do end @doc """ - `get_timer/1` gets a single Timer. + `get_timer!/1` gets a single Timer. Raises `Ecto.NoResultsError` if the Timer does not exist. @@ -53,6 +53,21 @@ defmodule App.Timer do """ def get_timer!(id), do: Repo.get!(Timer, id) + @doc """ + `get_timer/1` gets a single Timer. + + Returns nil if the Timer does not exist + + ## Examples + + iex> get_timer(1) + %Timer{} + + iex> get_item!(13131) + nil + """ + def get_timer(id), do: Repo.get(Timer, id) + @doc """ `list_timers/1` lists all the timer objects of a given item `id`. diff --git a/lib/app_web/controllers/api/item_controller.ex b/lib/app_web/controllers/api/item_controller.ex index 3b4994ff..1ab2c0f4 100644 --- a/lib/app_web/controllers/api/item_controller.ex +++ b/lib/app_web/controllers/api/item_controller.ex @@ -3,22 +3,24 @@ defmodule AppWeb.API.ItemController do alias App.Item import Ecto.Changeset - def show(conn, params) do - id = Map.get(params, "id") - - try do - item = Item.get_item!(id) - json(conn, item) - rescue - Ecto.NoResultsError -> - errors = %{ - code: 404, - message: "No item found with the given \'id\'." - } - - json(conn |> put_status(404), errors) - - Ecto.Query.CastError -> + def show(conn, %{"id" => id} = _params) do + case Integer.parse(id) do + # ID is an integer + {id, _float} -> + case Item.get_item(id) do + nil -> + errors = %{ + code: 404, + message: "No item found with the given \'id\'." + } + json(conn |> put_status(404), errors) + + timer -> + json(conn, timer) + end + + # ID is not an integer + :error -> errors = %{ code: 400, message: "The \'id\' is not an integer." diff --git a/lib/app_web/controllers/api/timer_controller.ex b/lib/app_web/controllers/api/timer_controller.ex index 3fff2786..8de31491 100644 --- a/lib/app_web/controllers/api/timer_controller.ex +++ b/lib/app_web/controllers/api/timer_controller.ex @@ -10,22 +10,24 @@ defmodule AppWeb.API.TimerController do json(conn, timers) end - def show(conn, params) do - id = Map.get(params, "id") - - try do - timer = Timer.get_timer!(id) - json(conn, timer) - rescue - Ecto.NoResultsError -> - errors = %{ - code: 404, - message: "No timer found with the given \'id\'." - } - - json(conn |> put_status(404), errors) - - Ecto.Query.CastError -> + def show(conn, %{"id" => id} = _params) do + case Integer.parse(id) do + # ID is an integer + {id, _float} -> + case Timer.get_timer(id) do + nil -> + errors = %{ + code: 404, + message: "No timer found with the given \'id\'." + } + json(conn |> put_status(404), errors) + + timer -> + json(conn, timer) + end + + # ID is not an integer + :error -> errors = %{ code: 400, message: "The \'id\' is not an integer."