Am I naming my GenServer functions correctly?

It’s my first time using GenServer for Elixir and I am managing to run it but I am getting warnings for my function arity/name even though they don’t override etc. It’s better to explain with code, here’s my module:

defmodule Voicer.UserCache do
  use GenServer
  alias Voicer.User

  # Client API

  def start_link, do: GenServer.start(__MODULE__, %{}, name: __MODULE__)

  def add_user(user), do: GenServer.cast(__MODULE__, {:add_user, user})

  def get_user(username), do: GenServer.call(__MODULE__, {:get_user, username})

  def remove_user(username), do: GenServer.cast(__MODULE__, {:remove_user, username})

  def list_users(), do: GenServer.call(__MODULE__, :list_users)

  # Server

  def init(users), do: {:ok, users}

  def handle_cast({:add_user, %User{username: username, key: key}}, users) do
    users = users |> Map.put(username, key)
    {:noreply, users}
  end

  def handle_call({:get_user, username}, _from, users) do
    return = if Map.get(users, username) == nil, do: nil, else: %User{username: username, key: Map.get(users, username)}
    {:reply, return, users}
  end

  def handle_call(:list_users, _from, users) do
    return =
      Enum.map users, fn({username, key}) ->
        %User{username: username, key: key}
      end

    {:reply, return, users}
  end
  
  def handle_cast({:remove_user, username}, users), do: {:noreply, Map.delete(users, username)}

end

I get an arity warning on the :remove_user and :list_users functions:

warning: clauses with the same name and arity (number of arguments) should be grouped together, “def
handle_cast/2” was previously defined (lib/voicer/cache/user_cache.ex:21)
lib/voicer/cache/user_cache.ex:31

warning: clauses with the same name and arity (number of arguments) should be grouped together, “def handle_call/3” was previously defined (lib/voicer/cache/user_cache.ex:26)
lib/voicer/cache/user_cache.ex:33

Am I naming/setting up them correctly/practically?

It’s just asking you to group functions of same name and arity together.

In this case it’s because having handle_cast/3 then 2 clauses of handle_call/2 and then handle_cast/3 again.

3 Likes

Oh, I interpreted the warning error differently, thanks!

Also you should make your handle_casts handle_calls. I think a good rule of thumb is that if you aren’t certain you need it to be a cast, it should be a call.

I don’t think so…

Whenever you do not want to return a value to the caller or it is not necessary for the caller to wait for the sideeffect to happen, then a call is enough.

2 Likes

Here’s how I feel about it:

  1. it makes your apis unified and you don’t have to check/think about your implementations to know moving forward.

  2. even if they don’t now, if your implementation should change in the future to something more complex that could incur a crash in your genserver, your calling process should probably crash when that happens, too. If it truly doesn’t care you can implement a crash handler or wrap it in a task.

  3. probably not a problem for most people, but backpressure management.

Point being, caller may care about completion. If you make it a call, there are options to ignore, if you make it a cast, it’s harder to go back on that choice.

1 Like

One small comment: it is much better if the name of your start function is the same as the name of GenServer start function it calls:

def start_link(), do: GenServer.start_link(__MODULE__, %{}, name: __MODULE__)

def start(), do: GenServer.start(__MODULE__, %{}, name: __MODULE__)

While it is of course not an error to do as you have done it will make it a little more difficult for users to understand what is going on without checking the code.

6 Likes

The real caveat of cast is that that it will return :ok even if the destination process doesn’t exist.

I agree with @Nobbz though, it’s best to understand the differences between handle_call, handle_cast, and even handle_info and choose the one that best matches your use case.

In the case of @ZastrixArundell, if you’re just learning to use GenServer it’s valuable to use both calls and casts to gain a better understanding of each.

3 Likes