Is it bad to use :not_found in a non-web module?

I have this function in a Repo:

def exists?(query, opts \\ []) do
  query = from q in query, select: true, limit: 1
  case !!one(query, opts) do
    true -> {true, :repo_resource_exists}
    false -> {false, :repo_resource_does_not_exist}
  end
end

That is used in a non-web-specific context module to see if a particular resource (a support request) exists:

def support_request_exists?(id) do
  query = from r in SupportRequest, where: r.id == ^id
  Repo.exists?(query)
end

And this code, in the same non-web-specific context module, uses that in a with:

def store_support_request_attachment(attachment, support_request_id) do
  #...

  with(
    {true, _} <- support_request_exists?(support_request_id), # Existence checked here
    :ok <- File.mkdir_p(destination_directory),
    :ok <- File.cp(attachment.path, destination_path)
  ) do
    {:ok, "#{support_request_id}:#{attachment.filename}"}
  else
    {false, :repo_resource_does_not_exist} -> {:error, :not_found} # Bad?
    # {false, :repo_resource_does_not_exist} -> {:error, :repo_resource_does_not_exist}
  end
end

In the else block, when the support request doesn’t exist I’m currently returning {:ok, :not_found}, which means this function in the fallback controller is used and results in a 404 page, which is what I want to occur:

def call(conn, {:error, :not_found}) do
  conn
  |> send_resp(:not_found, "")
end

However, I wonder if using :not_found in my app’s theoretically not-limited-to-web code is “bad”. The phrase ‘not found’ does seem to work for this in a non-web context, but is the :not_found atom specific to its HTTP equivalent response code, meaning it shouldn’t be used here?

Should I instead use the commented-out line and add the following function in the fallback controller?

def call(conn, {:error, :repo_resource_does_not_exist}) do
  conn
  |> send_resp(:not_found, "")
end

I’m a beginner and I don’t know whether or not this is an overly anal question. Sorry if so! :slight_smile:

If :not_found makes perfect sense in your domain then their is not really any problem to use it.

I would suggest adopting the :ok/:error convention (found all over elixir and erlang) most of the time. The reason is mostly just a description so not found works

2 Likes

Okay, great. Thanks.


I’ve had another think about function names and return values and now have the following.

Repo

def exists?(query, opts \\ []) do
  query = from q in query, select: true, limit: 1
  !!one(query, opts)
end

Context module

This function is now called confirm_..., meaning (in my mind, at least) that an :ok or :error is suitable as opposed to true or false.

def confirm_support_request_exists(id) do
  query = from r in SupportRequest, where: r.id == ^id
  case Repo.exists?(query) do
    true -> {:ok, :found}
    false -> {:error, :not_found}
  end
end

The with no longer requires an else block.

def store_support_request_attachment(attachment, support_request_id) do
  # ...

  with(
    {:ok, _} <- confirm_support_request_exists(support_request_id),
    # ...
  ) do
    {:ok, "#{support_request_id}:#{attachment.filename}"}
  end
end

As before, the existing fallback controller that matches {:error, :not_found} is used.

So I plan to stop thinking about it and get it committed now. :blush: