Best Practice: Extending default Context Functions

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

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

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

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

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