@spec best practices when having defdelegates

Questions / Help
,
1

ElixirForum

Hello, my beloveds, I was trying to reach in the best practices with some of my work colleagues @liryanne and @tom335, but we were not able to find an answer to this question. In this way, I would like to ask your opnion about a codding pattern that I could not find anywhere.

When using defdelegate and writing the needed @spec to my methods, where is the correct place to set these @specs?

Example:

ModuleA:

defmodule ImplementationModule do
  @spec inspect_data(data :: any()) :: {:ok, data :: any()}
  def inspect_data(data) do
    IO.puts("Inspecting_data...")
    {:ok, IO.inspect(data)}
  end
end

ModuleB which is the public interface for the internal ModuleA (and in a real case, lots of other internal modules):

defmodule DelegateModule do
  @spec inspect_data(data :: any()) :: {:ok, data :: any()}
  defdelegate inspect_data(data), to: ImplementationModule
end

IMHO, it gets redundant re-writing the @spec twice, this is the pattern I’ve been currently using, because this is my team’s project pattern, but I really would love to know what would be the best recommendation for this scenario

6 Likes
2

As described by @sallaumen, we’ve the feeling that the specs should only appear once, in the actual implementation; however it can be useful to replicate the specs in the module which delegate the same methods, in terms of documentation or even when publishing an external API, for example. It would be great to hear other use cases on similar situations.

2 Likes
3

I could see this being tricky - defdelegate takes specific steps to not have a compile-time dependency on the targeted module, but the only way it could fetch a @spec is from the compiled target…

6 Likes
4

This is a great question. A minor enough quibble perhaps, but it certainly feels redundant to add specs twice, so to speak.

1 Like
5

I agree that this is a great question. Maybe it being bumped now will elicit some more thoughts and opinions.

Here’s why I think that duplicating the specs is correct:

defdelegate is merely a convenient way of generating a function, but the fact that the call is being delegated should be invisible to the caller. The spec above the defdelegate is the public contract – from a compatibility standpoint, that’s the one you want to maintain. From the caller’s perspective, these are equivalent:

@spec inspect_data(any()) :: {:ok, any())
defdelegate inspect_data(data), to: ImplementationModule

# same as

@spec inspect_data(any()) :: {:ok, any()}
def inspect_data(data), do: ImplementationModule.inspect_data(data)

If someone changes the spec in the private implementation, I don’t want that to silently “leak” into the public API. For instance, let’s say that we want ImplementationModule.inspect_data/1 to stop returning a tuple, so it becomes:

defmodule ImplementationModule do
  @spec inspect_data(any()) :: any()
  def inspect_data(data) do
    ...
  end
end

I’d want dialyzer to yell at me! Hey, your public module’s spec returns a tuple, but the function it’s calling doesn’t! Now I have to make an explicit decision: Am I changing the public contract (a breaking change), or am I going to wrap it in a way that maintains the previous behavior?

@spec inspect_data(any()) :: {:ok, any()}
def inspect_data(data) do
  {:ok, ImplementationModule.inspect_data(data)}
end

If the public module implicitly “delegated” its spec as well, it is now fully exposing what should be an implementation detail. Duplicating the specs requires that you make a conscious decision if the implementation spec changes and that, in my opinion, makes it the correct option.

9 Likes
6

Really liked your point of view, thinking about the relation between the defdelegate function and the public function that it is delegating to, which has to keep consistent and by adding @spec on both dialyzer can help us with it too!

Thx for the answer @zachallaun

1 Like
7

I wonder if Recode plus Sourceror could be used to generate type specs for defdelegate functions automatically. Then as part of your CI process you could check if those specs have have changed. Although if you did want to ensure that the types stayed separate you’d have to note that somehow. Maybe this would be hard to make work but it’s an interesting train of thought.

There’s some interesting parallels to how GitHub - zachallaun/mneme: Snapshot testing for Elixir works

8

It wouldn’t be hard at all to find uses of defdelegate and automatically insert any specs associated with the function being delegated to, but I’m not totally following re: the CI process. Do you mean a CI that runs Dialyzer? That should definitely catch any issues if one spec changes without the other changing.