jeremy.owensboggs
Defoverridable disclaimer to use it with care - but why?
While perusing the defoverridable documentation, one of my teammates pointed out the disclaimer:
Use
defoverridablewith care. If you need to define multiple modules with the same
behaviour, it may be best to move the default implementation to the caller, and check if a callback exists viaCode.ensure_loaded?/1andfunction_exported?/3.
The disclaimer doesn’t explain why this is. What is the reason for this disclaimer? Why is it be preferable to query the module for an implementation, rather than use a defoverridable? Is it a performance thing? Readability thing? Other?
Most often I see defoverridable used with a using macro - does that come into play with this disclaimer?
Most Liked
josevalim
The issue with defoverridable are two:
-
It leads to broad interfaces because you can define default functions that are “inherited” by everyone. While this is also possible with callbacks, the fact a callback is optional means you cannot rely on it as part of your API. This is part of an overall principle were you generally want to keep your interfaces narrow.
-
Overridable functions come from meta-programming, which is harder to debug, and an overriden implementation can call their previous definition using
super, which is not something you can turn on or off and creates additional coupling, regardless if it is intentional or not.supercan also cascade, which leads to more issues during trouble-shooting.
There are situations were defoverridable is preferred and I think @slouchpie gave a good example, where you don’t want to invoke Code.ensure_loaded? on every time. But for process-based behaviours or behaviours with clear entry-points, I’d recommend optional callbacks. I will improve the documentation.
As I said at the time, saying there are several instances does not move the needle, because the Elixir team doesn’t know where they are. My biggest concern is that, without quantifying them, we may be simply repeating information which is no longer true. Especially because, if there are examples such as this discussion, then I am certain they will be addressed.
EDIT: Improve docs for defoverridable and behaviours · elixir-lang/elixir@38f460e · GitHub
mudasobwa
Normally, the implementations are called through the dependency injection and they don’t need any use clause. Like this
defmodule TestBeh do
@callback ok :: :ok
@callback ko :: :ko
@optional_callbacks ko: 0
@default_impl Application.compile_env(:foo, :test_beh, TestImpl)
# default implementation
def ko, do: :ko
def ok(impl \\ @default_impl), do: impl.ok()
def ko(impl \\ @default_impl) do
impl = if function_exported(impl, :ko, 0), do: impl, else: __MODULE__
impl.ko()
end
end
slouchpie
Imagine we want to use a behaviour like this:
defmodule NameBehaviour do
@callback name :: String.t()
@optional_callbacks [name: 0]
end
APPROACH 1
With defoverridable it would be:
defmodule Greeter do
defmacro __using__(_opts) do
quote do
@behaviour NameBehaviour
defdelegate name, to: unquote(__MODULE__)
defoverridable name: 0
end
end
def name do
"stranger"
end
def greet(module) do
"Hello " <> module.name()
end
end
APPROACH 2
Without defoverridable (as docs suggest) it would be:
defmodule Greeter do
defmacro __using__(_opts) do
quote do
@behaviour NameBehaviour
end
end
def greet(module) do
if Code.ensure_loaded?(module) and function_exported?(module, :name, 0) do
"Hello " <> module.name()
else
"Hello stranger"
end
end
end
Personally, I think “Approach 1” looks much better.
Surely it is not good to have this Code.ensure_loaded? and function_exported? runtime evaluation every time I call this function?
Update: function_exported? is inlined by the compiler but I don’t know if Code.ensure_loaded? is.
mudasobwa
There is no need to be sceptical. Since v1.14 IIRC, there was a lot of work on compiler to decrease coherence.
You might check it yourself by creating the project, with the two following files
❯ cat lib/test_beh.ex lib/test_impl.ex
defmodule TestBeh do
@callback ok :: :ok
end
defmodule TestImpl do
@behaviour TestBeh
@impl TestBeh
def ok, do: :ok
end
then run mix compile
❯ mix compile
Compiling 2 files (.ex)
Generated test_beh app
then change lib/test_beh.ex to add some function (playing the role of the default one) and run mix compile again.
❯ mix compile
Compiling 1 file (.ex)
Generated test_beh app
Remote calls were not triggering the recompilation of the file containing remote calls since Anno Domini.
slouchpie
I would also like some more information on this. The defoverridable approach looks a lot cleaner than the Code.ensure_loaded? approach.







