Idiomatic pattern matching without arity overloading. (Single function vs mutliple)

markusgod

Hi all, can you advise me how to better handle multiple handle_info in such code, or what I did is fine?
At first I was yelled at, but Credo linter, that I added a @doc to each handle_info and it was overwritten, I’ve merged them into a single doc. Now I think should I collapse this function with a single giant spec, I will expand it with more terms and behaviors, so to me it looks like keeping separate function is better, but @spec in docs looks a little bit crammed together, I would expect it to split out into multiple functions, but seems like it follows arity as with @doc

defmodule ServerAlpha.UserSession do
  @behaviour WebSock

  @moduledoc """
  The UserSession module implements `t:WebSock.impl/0` for `WebSockAdapter.upgrade/4`
  and handles the WebSocket connection.
  """

  @typedoc """
  The state of the UserSession.
  """
  @type t :: %__MODULE__{
          user_id: String.t() | nil,
          connected_at: DateTime.t()
        }
  @enforce_keys [:user_id, :connected_at]
  defstruct [:user_id, :connected_at]

  @impl WebSock
  @spec init(t()) :: {:ok, t()}
  def init(state) do
    {:ok, state}
  end

  @doc """
  Handles incoming messages from the WebSocket connection.
  """
  @impl WebSock
  def handle_in({_text, _opts}, state) do
    {:ok, state}
  end

  @doc """
  Handles messages sent to this process from `send/2`. This should be used to implement sending **to** the user.

  ## Messages
  - `{:send, message}` - Sends a message to the user
  - `:send_disconnect` - Sends a disconnect request to the user
  """
  @impl true
  @spec handle_info({:send, String.t()}, t()) :: {:push, String.t(), t()}
  def handle_info({:send, message}, state) do
    :logger.debug("UserSession received #{inspect(message)}")
    {:push, message, state}
  end

  @impl true
  @spec handle_info(:send_disconnect, t()) :: {:stop, :normal, t()}
  def handle_info(:send_disconnect, state) do
    :logger.debug("UserSession received disconnect request")
    {:stop, :normal, state}
  end
end

9 comments

#learning-elixir

1 271 9

2024-11-03 17:55:27 UTC

First 9 of 9 Posts

dimitarvp

You don’t have to duplicate @impl true and @spec for each variant of the same function. Just once is enough for the compiler and Dialyzer.

For the rest, the answer is always “it depends” but I prefer to have just one handle_info and then call several different private functions depending on which arm in the case statement you’re in. I feel it gives me the opportunity to make the code more self-documented by naming the private functions properly.

Again though, just a preference depending on the size of the code, and how unreadable it would be otherwise.

Post #1

markusgod

I haven’t thought about having a private functions, that may work for me.
Regarding spec. I want to specify that my function accepts a subset of what specified in a callback definition. Instead of term() specific terms {:send, string.t()} and etc.
Maybe I can create a type that will encapsulate all allowed terms, and use it in a single spec, but that will mix up a return value types.
Or maybe spec per function is good enough.

Post #2

dimitarvp

Very good point RE: more concrete specs per function variant.

Post #3

al2o3cr

What are you aiming to accomplish with adding @spec to handle_info? It’s not something that’s going to be called directly, and tools like Dialyzer aren’t able to “see” the typing through apply.

Regarding the multiple clauses, this is one of the spots where the new set-theoretic type machinery is aiming to help - for instance, negate in the example can only currently be given the spec negate(integer | boolean) :: (integer | boolean) which fails to completely capture the relationship between the input type and the return type.

Post #4

xlphs

I think it’s due to this credo rule Credo.Check.Readability.Specs — Credo v1.7.19 which seems to blindly require all public functions to have typespecs.

Post #5

sodapopcan

That’s really strange: it explicitly states that callbacks should have specs. It should really ignore anything that has an @impl. What would the thinking there be? If you want it for documentation purposes you can look at the behaviour otherwise it’s just noise IMO. Am I missing something?

Post #6

markusgod

I’ve kind of got lost in types for a bit. I’ve mostly hoped that it would be good documentation for myself from the future.
After a walk I understood that it will handle messages from ‘send’ anyway, and there is no way to annotate to what messages are being sent. Like ‘send’ that can suggest data from handle’s signatures.

Post #7

dimitarvp

It’s still very valuable information for future maintainers.

It’s 50/50 and I like your approach but you could also do it like this as you alluded to earlier and I don’t think a lot of information will be lost:

@type message_request :: {:send, String.t()}
@type disconnect_request :: :send_disconnect

@type message_response :: {:push, String.t(), t()}
@type disconnect_response :: {:stop, :normal, t()}

# ...

@impl true
@spec handle_info(message_request() | disconnect_request(), t()) ::
    message_response() | disconnect_response()

Sure, some information is lost – like is it OK to have a message_request in parameters but disconnect_response as a return value is what I’d be a little grumpy about – but expressing types like this still increases readability and you can also put a short comment saying that a message response is only returned when message request is passed and the same for disconnect and others.

Ultimately though, your way is still fine IMO. And I might prefer it in some cases.

Post #8

sodapopcan

Ya you can go for it if you want for sure, it’s just weird that Credo thinks it’s a good idea as the type info is still available to you and otherwise isn’t accomplishing anything as far Dialyzer goes.

Post #9

— All posts loaded —