Best Practice: Extending default Context Functions

Phoenix Forum Questions / Help
1

Hi all,

I have a solution to what I want to achieve, I am just not sure if it’s “the Elixir way”.

Let’s start with this Phoenix Context module/function, straight out of the generator:

defmodule ExamplePlain do
  @doc """
  Deletes an Item.

  ## Examples

      iex> delete_item(item)
      {:ok, %Item{}}

      iex> delete_item(item)
      {:error, %Ecto.Changeset{}}

  """
  def delete_item(%Item{} = item) do
    Repo.delete(item)
  end
end

What I would like to do is add functionality here (to be specific: deleting some caches) without changing the return values. The argument could be made that additional method calls should be defined separately and chained when used like item |> delete_item |> delete_item_caches. However I would like this to be atomic, meaning the programmer does not need to remember to delete caches whenever she calls delete_item. Same goes for creating and updating items.

My solution:

  def delete_item(%Item{} = item) do
    case Repo.delete(item) do
      {:ok, item} ->
        delete_item_caches(item)
        {:ok, item}

      {:error, changeset} ->
        {:error, changeset}
    end
  end

It does the trick but it feels clumsy. Any ideas for improvements or how to better approach such situations?

Thanks for your help
Eric

2

I think your approach is fine. You might also check that delete_item_caches(item) returns what you expect. You can also use with:

def delete_item(%Item{} = item) do
  with {:ok, %Item{}} <- Repo.delete(item),
        :ok <- delete_item_caches(item), do: {:ok, item}
end
1 Like
4

Just a minor observation:

defmodule Demo do
  defp foo(x) when x > 0,
    do: {:ok, x}
  defp foo(x),
    do: {:error, x}

  def bar(item) do
    case result = foo(item) do
      {:ok, item} ->
        item = item + 1
        IO.puts("success")
        {:ok, item}     # {:ok, item} signals that it IS DIFFERENT from match
      {:error, _} ->
        result          # result IS {:error, other} - reuse
    end
  end

  # similarly

  def baz(item) do
    with result = {:ok, item} <- foo(item),
         :ok <- IO.puts("success"),
         do: result     # reuse previously matched result
  end
end

IO.inspect Demo.bar(1)
IO.inspect Demo.bar(-1)
IO.inspect Demo.baz(1)
IO.inspect Demo.baz(-1)

YMMV

I would like this to be atomic

if delete_item_caches reaches out to another process there may still be the possibility of a race condition if other processes use that caching process independently.

1 Like
5

What’s the point of using result? Unlike in erlang, there is no guarantee it wasn’t reassigned to something else.

6

Given that foo already created the tuple it seems redundant to create the same tuple in baz all over again.

Your code is a bit different as conceptually you are creating a “new” tuple with the original item argument, rather than returning the tuple that is coming from Repo.delete(item) (which the OP’s code uses).

2 Likes