Fixing default behaviour callback implementations

[Christopher Keele]

Hey all! I'd like to talk about my biggest frustration with Elixir as she is wrote: packaging default callback implementations with behaviours.

The Problem

Today this is the almost universally used idiom:

defmodule Custom.Behaviour do

  @callback foo :: atom

  defmacro __using__(_opts \\ []) do

    quote do

      @behaviour unquote(__MODULE__)

      def foo, do: :bar

    end

  end

end

That is, creating a __using__ macro in the behaviour module with a quote block that just defines all the default implementations for the callbacks. I firmly believe this is a bad pattern:

• There is no concrete implementation of them so they can only be tested through a layer of indirection, defining new modules within the test suite
• If the project is a library that provides a behaviour it doesn't itself use, then tests are the earliest opportunity to get useful compiler warnings about those default implementations
• Errors raised in these default implementations have an inscrutable backtrace
• The way the default callback functions work have no good place to be documented
• The default implementation is far from its callback declaration and easy to get out of sync

The last point is remedied by the new @impl annotation but I don't see that being used as often as I'd like.

I also rarely see the macro-generated implementations of the callbacks provide their own documentation, and often the behaviour's moduledoc doesn't go into depth or cover the workings of all default implementations.

Furthermore, this situation is a lot of newcomer's first exposure to writing their own macros, and this idiom teaches them the opposite of good macro practices, such as:

• not writing them unless necessary (this really is a boilerplate-common scenario)
• keeping them slim (the macro grows linearly with number of callbacks, bad for wide-surface-area behaviours)
• having macros make as many calls to local functions as possible so they can be tested (I've never seen this done for this default-behaviour use-case)
  

The Fix

My proposal is to eliminate usage of this pattern, by providing a better way of accomplishing the same thing, preferably within core.

That's a whole discussion on its own, but I would also like to suggest what I think is a superior pattern for handling this we could use as the recommended happy path: defining the default implementation of a callback on the behaviour module itself, immediately alongside its callback declaration, and creating a __using__ macro that instead generates a delegate to this implementation. This has many virtues and fixes all of my frustrations:

• It gives a concrete implementation of these functions so they can issue compiler warnings
• and be documented
• and be tested
• The implementation is alongside its callback definition to stay sync (indeed, the impl's @spec should always mirror the @callback above it)
• Errors will reference the offending line within the implementation instead of the __using__ macro call-site
• Using delegates theoretically reduces compiled code size, assuming the behaviour is used more than once and the implementations aren't oneliners

The main downside is that this pattern requires a little more boilerplate (doing the delegation), and sadly the delegates themselves are far from their implementations and can still easily get out of sync, although they are more likely to raise an error in this case.

By creating a macro that does this boilerplate correctly for you, this would be an entirely positive change, and if we put it in core, it will hopefully supplant the old idiom as people update their source.

This is the heart of my proposal: let's provide a better way to do this out of the box, with as little as boilerplate as possible.

One Implementation

To instigate discussion, I have put up the crude implementation of this feature I've been toying with in one of my libraries here. It's a module with a __using__ macro that will generate a __using__ macro for your behaviour modules.

It does more manual AST munging than I would like and steers clear of some of the harder questions of what to do with types and specs, simply copying the documentation over for functions and providing a delegate implementation. Ideally it would only do this for functions flagged as callbacks within the behaviour module, though my first take simply copies over all public functions.

It can be instructed to suppress documentation, and provide inline implementations rather than delegates. It sets the @behaviour for you, provides the @impl annotation for all functions, and sets them all as overridable. The behaviour module can still be used normally with @behaviour so applying these default implementations is entirely optional.

Other considerations

My particular approach (having a __using__ generate a __using__ macro for your behaviour) is certainly not the only way to go about this; defining which functions are default implementations within a behaviour could be done with an explicit macro, declaring that you want those default implementations inside your using module could be an explicit macro. I prefer this solution because it requires the least effort possible, using the __using__ mechanism to not even need explicit requiring, and therefore is more likely to be adopted.

This technique doesn't work for modules like GenServer that need a different API than their callbacks. For most common cases my approach is viable but the implementations could be tucked away inside a .Defaults namespace or something instead to support variant APIs.

I like the idea of un-deprecating and repurposing the Behaviour module to hold this feature, since its moduledocs could absorb the behaviours page, where I think it would be easier to find organically in the docs, just because I find the pages section often gets overlooked.

One might argue that having both "@behaviour FooBar" and "use FooBar" again could be confusing. This feature could be implemented instead as macros that take a behaviour module as an argument, appearing in Macro instead, or be provided as an unadorned defdefaults or something, though those seem less elegant to me and it would complicate the implementation (at least for inlining, where having an @on_definition hook in the behaviour module is essential).

But, this is all pretty subjective and ancillary to the main point: should we offer a better way to do this out of the box? It's quite possible I'm the only person who is driven crazy by the current vogue idiom.

[Paul Schoenfelder]

I'm not sure all of your criticisms are accurate. For one thing, I find that it is rather rare to provide default callback implementations for most behaviors - while there are exceptions, I think it's generally more common to have a "default" implementation of the behavior as a whole, for example, an in-memory implementation of a cache or data store as a default with other implementations typically hitting external equivalents. You could also place default implementations in the behavior  module itself and use defdelegate from the __using__ macro to have the default implementation delegate to the behavior. Error traces are very clear as long as the macro has `location: :keep` set, which is generally always the case. 

I don't disagree that having a way to perhaps ease the boilerplate for this pattern would be nice, but I also appreciate the explicit nature of how it works today. I'm not sure that the solution you described makes me feel like it's a clear win. Boilerplate isn't bad by definition, it can lend clarity when the structure of things is more explicit and standard - the more you hide, and the more magic there is, the more opaque it all becomes, and I think makes it harder to really understand.

Anyway, I'm interested to hear everyone elses thoughts, but I wanted to chime in with my two cents.

Paul

--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-co...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/3701e507-b002-4e05-9b54-c979f2d3275f%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Christopher Keele]

I find that it is rather rare to provide default callback implementations for most behaviors

To be fair, I have been working on a project on and off for over a year, that contains two very large behaviours, where all callbacks are mandatory, but a majority of them can offer a default implementation derived from a few core ones—but should be overridable for performance optimizations. In short, the worst case scenario for the current common pattern; I'm sure this has exacerbated my frustration beyond normal levels.

You could also place default implementations in the behavior  module itself and use defdelegate from the __using__ macro

This is the approach I was using for a while. The code in the gist I've linked to is essentially defining such a __using__ macro for you, for all of your callback functions, using a handrolled version of defdelgate that can also preserve documentation and optionally inline, instead of just delegating, the default implementations. Again, traits necessary to my usecase but ones that I think would nice to have in a boilerplate macro.

[José Valim]

My main concern is to introduce another mechanism where functions are dynamically added to modules. The issue with __using__ is that it is too generic but that it is also its blessing as we have a single entry point.

It is also worth saying that we should generally stop defining default implementations. We should use @optional_callbacks and function_exported? if the callback is truly optional. That's the direction we are moving GenServer, Supervisor and friends. See previous discussion here: https://elixirforum.com/t/behaviours-defoverridable-and-implementations/3338

José Valim

www.plataformatec.com.br

Founder and 

Director of R&D

--

You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.

To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/309216d9-81fb-4a83-9a8e-c7034d00d6ce%40googlegroups.com.

[Christopher Keele]

Haha, I think I posted on that discussion asking for this same thing back then, too.

My main concern is to introduce another mechanism where functions are dynamically added to modules.

That's a very important consideration I had overlooked entirely.

 

It is also worth saying that we should generally stop defining default implementations.

This is also a fair point. I have worked on some projects with very particular needs, where all callbacks are truly mandatory (to conform to an interface) and not optional, but it is also trivial to provide default implementations for most by leveraging a few others, and rarely but strongly desirable to override some of those defaults.

Clearly my experience in this arena has not been standard.

I suppose another approach to this would be to try to augment defdelegate with support for some of the features it lacks that caused me to write my alternate version, so that piecing this sort of thing together within a __using__ macro requires much less glue without adding much surface area to the language.

I'll look into the feasibility of instrumenting it with the option to copy @docs onto the delegate next time I'm frustrated enough to look into this again, and go from there.