Elixir: Support callbacks for private functions

[devinus]

Example: https://gist.github.com/df0078c4743b9e052dc6

Will produce warnings about unimplemented behaviour functions.

[yrashk]

Doesn't this sort of go against the "purpose" of private functions? I mean, they are "private" for a reason...

[josevalim]

Yup, agreed. Or you make the function public or remove it from the behaviour specification.

In the case above, it seems that post_decoder is an implementation detail of the decode mechanism. In theory, you don't need post_decoder at all, unless you are using the default decoder implementation. So I would argue post_decoder is not part of the json behaviour at all.

[devinus]

Seems fair. Maybe I should refactor the implementation or make pre_encoder public.

[wanderanimrod]

Wouldn't this be useful in the following case?:

I want to make sure that if ModuleA uses my BaseModule , ModuleA must define function f without also exposing f to other modules that might call ModuleA for other reasons.

I don't think requiring function f to exist as a callback should necessarily require one to make the function public to other modules other than BaseModule. Is there any other way to handle the case I have described above?

[NobbZ]

You weren't able to call f from anywhere but ModuleA so why force it to be there? From the outside you can't see any difference.

Also I do not really understand your usecase. Do you think of functions that are only accessible by a handfull of other modules? There is not such a thing in the BEAM, either a function is public for everyone or private and not usable at all.

But most of the functions defined as a callback aren't called by anything else then the defining module. Even though handle_cast in a GenServer implementor is public, you'd never call it manually, do you?

[wanderanimrod]

defmodule BaseModule do
  @doc """
  All modules that use `BaseModule` must implement this callback because `BaseModule`
  defines function `b/0` that calls `f/0`.
  """
  @callback f() :: atom

  defmacro __using__(_) do
    quote do
      # This line here ensures if a module uses `BaseModule` and doesn't define the `f/0` 
      # callback, a compile-time warning will be generated
      @behaviour BaseModule

      @doc """
      This function is the only function that other modules should call.
      It calls the callback `f/0` in order to do its work, but other modules
      should not know about this. `f/0` is an implementation detail of modules
      that use `BaseModule`
      """
      def b() do
        result_from_callback = __MODULE__.f()
        {:ok, result_from_callback}
      end
    end
  end
end

defmodule ModuleA do
  use BaseModule
  @impl true
  def f(), do: :callback_result
end

__Usage outside BaseModule__

This is good, and normal.

ModuleA.b() #=> {:ok, :callback_result}

This is not good.

ModuleA.f() #=> :callback_result

f/0 is callable on ModuleA as expected because it is public. BUT, because it is
just an implementation detail required by BaseModule, I don't want code outside
BaseModule to be able to call it.

Now, the normal way to hide "implementation details" is to make a function private. If I make the function f private, I can still call it within the b function because __using__ compiles both b and f into ModuleA.

defmodule ModuleA do
  use BaseModule
  @impl true
  dep f(), do: :callback_result
end

As expected, ModuleA.b still works:

ModuleA.b() #=> {:ok, :callback_result}

And as desired, ModuleA.f(which is an implementation detail) cannot be used outside BaseModule

ModuleA.f()
** (UndefinedFunctionError) function ModuleA.f/0 is undefined or private

__However__, I get two warnings at compile time that only go away if I make ModuleA.f/0 public again, which is undesirable because it is an implementation detail.

warning: defp f/0 is private, @impl is always discarded for private functions/macros
warning: undefined behaviour function f/0 (for behaviour BaseModule)

While it is true that GenServer modules require their callbacks to be public, it doesn't make exposing functions that are not meant to be used a good thing. We don't call GenServer's handle_call/3 directly because we have learned that they are not supposed to be called.

In the case of non-standard-lib modules (like the case I described above), it is clear that exposing implementation details just because they are callbacks results in confusing module APIs.

Is there a pattern that one can use to avoid this situation? If there is none, can we allow callbacks to be private functions?

[NobbZ]

The convention is to not document internal functions.

So keep f as a callback, document it as a callback, def it in the implementor and do not put any @doc around it. This way it won't show up in the generated docs and considered beeing an internal function not meant to get called from the outside world.

We didn't learn about GenServers callbacks that are the ones we shall not call, we learnt about handle_call isn't in Foos documentation, so it is not meant for public use.

[wanderanimrod]

Thanks for the response @NobbZ . The @doc on the callback was just meant to explain the code. I wouldn't have it in real code. So the @doc there shouldn't not take us off-track.

About your comment on GenServer, what you are saying is that looking at a function that is public on a module doesn't mean it is meant to be used outside that module. You seem to suggest that the presence or absence of a @doc annotation should take precedence over def or defp when deciding whether a function is meant to be used or not. I know that the absence of a @doc annotation on a function is widely interpreted to mean that function is not to be used, but I have never come across anything in the language docs (or anywhere else) saying @docs absence or presence is the true basis for determining if a function should be used outside a module.

__What I am saying is that it would be much simpler if a module's public functions and its API were the same thing__. That is, it would be clearer for users of a module to deduce it's API by looking at its public functions without having to eliminate public functions have @doc false on them.

[josevalim]

@wanderanimrod the reason why we don't allow private callbacks is to avoid exactly the code you wrote. You should think about the public contracts between the modules instead of trying to hide functions that are only exposed via code injection. The proposed code violates our best practices in writing maintainable macros since it by definition requires injecting logic in the user code. It also violates the rule that every function you generate in a user module should be part of a callback.

In Ecto, we solve this problem by having two different behaviours: Ecto.Repo, which is the public API, and Ecto.Adapter, which is the API called by Ecto.Repo. The contracts then become clearer on both sides and the user doesn't need to know about Ecto.Adapter unless they need to implement one. In other words, prefer to split the concerns over more modules instead of trying to fit everything in one.

[josevalim]

Also, did we meet at ElixirConf? :)

[wanderanimrod]

Thanks for the response @josevalim. Yes, we did meet at ElixirConf 😄.

The proposed code violates our best practices in writing maintainable macros since it by definition requires injecting logic in the user code

This is a very confusing statement to me. Macros inject code in the places they are used. At least, that is my current understanding. This statement only makes sense if you differentiate between a macro _transforming the code you give it (e.g. ExUnit.Case's test)_ from a macro _injecting completely new code in a user module (e.g. BaseModule injecting function b/0)_.

It also violates the rule that every function you generate in a user module should be part of a callback.

This looks like a good rule to follow, but this is the first time I am encountering it. I couldn't find it in the best practices manual you shared. If I followed that rule, I wouldn't be injecting function b/0 into ModuleA using the macro.

Thanks @NobbZ and @josevalim for the responses. They have helped me think hard about the choices that have brought me to this point 🤔 .

[josevalim]

This is a very confusing statement to me. Macros inject code in the places they are used. At least, that is my current understanding.

In the document I linked, search for "For example, instead of writing a macro like this". It will show an example that explains that any function generated in the user module should imply trigger an external function that has the logic, in order to minimize the generated code.

You can also see how Ecto.Repo does it: https://github.com/elixir-ecto/ecto/blob/master/lib/ecto/repo.ex :)

[michalmuskala]

My additional 2 cents. In my experience, a need for private callbacks is usually a result of trying to emulate OO inheritance through the use of macros and code injection. In many cases, we have better ways of achieving similar functionality that results in better and more idiomatic code.

Two primary ways are higher-order functions and callback modules. In case there's only a handful of those functions to "specialize" adding one generic function accepting a fun specializing the behaviour is usually a much better solution. The user of the library can then decide to either call it inline or if they use it often, to create a wrapper function always passing their desired anonymous function.
A generalisation of a higher-order function for places that need many different funs are callback modules and behaviours - they allow defining multiple, named functions and establishing a contract for the module.