Hello @mark_lemberg and welcome !
Could you elaborate on why you agree with Dave and how you would apply his opinion to the piece of code I provide in the question?
Fl4m3Ph03n1x
dimitarvp
I think looking for universal rules would not ever work and is a distraction in general. I’ll post one of mine though since it proved itself as a time saver many times.
I draw the line at how complex would a file be for me to parse by eyeballing. For most simple(-ish) GenServers I leave everything in one file. This gives me the peace of mind that I can easily find whatever I need for functionality XYZ in file xyz.ex.
It is only when a module and its responsibilities grow big that I’d look into splitting it into multiple files. And even then I’d analyse probable feature envy and refactor into several new modules. Very rarely have I split a singular module into several files (interface / implementation / runtime artifacts).
2
peerreynders
Another goal of the article seemed to be encapsulation of data inside the process.
In objects the advantage of encapsulation is two-fold:
- only the object’s methods can mutate the internal data
- client objects couple with the interface (API), not the structure of the internal data.
In Elixir values are immutable so mutating a data structure is not an issue (though a name can be rebound). However if clients have to access the data structure directly they become coupled.
When it is desirable to treat the data structure as opaque then all access needs to go through module functions to decouple the caller from the internal structure. While this type of “encapsulation” isn’t enforced, it is often good enough.
So in the article it was surprising to me that the API was modified for the server version when in fact all that was needed was to go from:
@type names() :: map()
@spec new() :: names()
@spec lookup(n, k) :: {:ok, v} | :error when n: names(), k: term(), v: term()
@spec store(n, k, v) :: n when n: names(), k: term(), v: term()
to
@type names() :: pid()
@spec new() :: names()
@spec lookup(n, k) :: {:ok, v} | :error when n: names(), k: term(), v: term()
@spec store(n, k, v) :: n when n: names(), k: term(), v: term()
1
pragdave
Author of Programming Elixir
I wouldn’t particularly argue against the code you have. I’m not sure I’d have the interface module unless I knew there’d be more implementations, and I’d do the types a little differently.
@type via_tuple_without_value :: {:via, module, {module, {module, any}}}
@type via_tuple_with_value(val) :: {:via, module, {module, {module, any}, val}}
@type via_tuple :: via_tuple_without_value | via_tuple_with_value(any)
@spec via_tuple(tuple) :: via_tuple_without_value
@spec via_tuple(tuple, any) :: via_tuple_with_value(any)
If the main module has just the API, then adding the extra behaviour seems to obscure things to my eye,
To your point about splitting APIs, what you have here is exactly what I describe. The external Registry is the implementation (only without the useless name 
) and this file is the API that delegates to it. Yes, it adds a little along the way, but not so much that it obscures its nature.
What I’m less keen on is the start_link/child_spec magic in here. As far as I can tell, the API code would still run fine if these two were moved into a separate module: in fact the API would run fine any time there is a Registry process called Clint.ProcessRegistry running. So I might do a little refactoring:
defmodule Clint.ProcessRegistry do
@registry_name __MODULE__
@type via_tuple_without_value :: {:via, module, {module, {module, any}}}
@type via_tuple_with_value(val) :: {:via, module, {module, {module, any}, val}}
@type via_tuple :: via_tuple_without_value | via_tuple_with_value(any)
@spec lookup(tuple) :: [{pid, any}]
def lookup(key),
do: Registry.lookup(@registry_name, key)
@spec update_with_value(tuple, any) :: {any, any} | :error
def update_with_value(key, new_value),
do: Registry.update_value(@registry_name, key, fn _old_value -> new_value end )
@spec via_tuple(tuple) :: via_tuple_without_value
def via_tuple(key),
do: {:via, Registry, {@registry_name, key}}
@spec via_tuple(tuple, any) :: via_tuple_with_value(any)
def via_tuple(key, value),
do: {:via, Registry, {@registry_name, key, value}}
#############################################################################
defmodule Server do
@registry_name Clint.ProcessRegistry
def start_link,
do: Registry.start_link(keys: :unique, name: @registry_name)
def child_spec(_) do
Supervisor.child_spec(
Registry,
id: @registry_name,
start: {__MODULE__, :start_link, []}
)
end
end
end
I haven’t run this, so it might be total BS.
I’m not keen in the duplication of the registry name between the two modules, but I couldn’t quickly come up with an alternative which wasn’t even worse
The two things I prefer about this solution are:
-
The API and the server stuff are separate, so there’s no need for a behaviour module, and
-
The use of
__MODULE__is disambiguated: before it was used both as a process name and as the module passed to the supervisor, which made it harder for my few remaining neurons to work out what was going on.
But…
Honestly, what’s important here isn’t whether a particular structure is right or wrong. The most important thing is that we’re discussing it, and thinking about it.
Nicely done.
Dave
8
Fl4m3Ph03n1x
Thank you so much for your criticism.
It is clear from your post I have a long way to learn about giving my opinions, you share yours in such a nice way that in comparison I look like Hitler when sharing mine 
I quite like the idea of separating the Server from the API, but at the same time there is that pesky indirection argument people like to use against this methodology.
I can see the value in both, I just need to find the line between when it makes sense to use one model or the other.
Once again, thanks for your reply!
0
sasajuric
Author of Elixir In Action
IMO, starting required processes is the part of the API of an abstraction, just like constructor is the part of the API of a class, or new is the part of the API of some data abstraction (e.g. Map).
With the proposed code split, in order to use Clint.ProcessRegistry, I need to first start Clint.ProcessRegistry.Server. In my view this makes the usage and the subsequent reading experience more complicated. Having to start Foo in order to use Foo seems more intuitive to me.
Of course, there are some cases where it’s worth splitting the abstraction API, for example if the API becomes too large and covers a bunch of semi-related concerns. In such scenarios, I’d prefer to have the top-level module, e.g. Foo providing the childspec, and then have other API concerns exposed via “submodules”, such as Foo.ApiConcern1, Foo.ApiConcern2, … That way, if I want to use different aspects of Foo, I have to start Foo, which, again, seems more intuitive than having to start Foo.Server.
Either way, the exact approach I’d prefer is IMO highly dependent on the context. In this simple case, given the code size, I’d definitely keep the start API (i.e. the childspec) together with the rest of the API.
5
pragdave
Author of Programming Elixir
Thanks for highlighting this, because I think it’s at the core of much of my thinking.
The Beam is a pretty unique environment, and it makes us think about many things differently. One major difference is in the way we have to think about process lifecycle as well as process functionality.
Take this particular example. We have a global registry with an API (lookup, update, and so on). In most other languages, that would be the end of it. But on the Beam the conventional way to have global state is to have a named process, and so we now have lifecycle code that starts this process.
But I suggest that this code (the start_link and child_spec) are not part of the API of the Registry. They are part of the lifecycle, the scaffolding needed to get the Registry working on the Beam. 99% of the time, it will only be invoked as a tuple passed as a child spec to some top-level supervisor. It never gets called by people using the module.
So that’s at the root of my concern. We mix traditional APIs and BEAM lifecycle management code into the same bucket of functions. And, as a result, I think we often conflate supervision with functionality. But the two are separate: the supervision tree is an expression of process lifecycle, and not process functionality.
3
sasajuric
Author of Elixir In Action
I regard the fact that these funs are (usually) not called directly as a mechanical detail. To me they are still first and foremost a part of the same API, because these functions cover one aspect of how we use the abstraction. IMO starting a thing is a part of using the thing, and so it’s a part of the API of that thing.
Separating into two modules doesn’t change that - the Registry docs still need to instruct me to start Registry.Server before I can use Registry functions. So the API surface remains the same, except now it’s split across two modules. Now I need to read docs of both modules because they cross-reference themselves (e.g. docs for Server explain how some childspec/start_link option affects the behaviour of some function in the “API” module and vice-versa). I personally find this confusing.
I can agree that starting might be considered as a separate API concern, so splitting these two concerns might sometimes be valid, but IMO that’s not the case in this simple example. I feel that such split here brings more usage/reading distractions with little to no practical gain.
3
pragdave
Author of Programming Elixir
If it’s a “mechanical detail” then I’d argue that it’s definitely something that isn’t important when reading. It’s just housekeeping noise.
That’s why I have the ability in the Component library to have globals started automatically.
But it’s all just style, and there’s nothing but time and experience to judge which is preferable. And indeed, as you say, there’s no one right answer.
I push these points, though, because the community seems to be very happy to continue to write code as it has been written for the last 20 years, and that sets off little alarm bells in the agile part of my brain. Unless we try new ways of doing things (rather than just talking about them) we’ll never learn if the current way is indeed the best. I don’t want to debate. I want to code
2
sasajuric
Author of Elixir In Action
The style might not have changed much within BEAM languages (it actually has, but more on that later), but I feel that this style is mostly unknown to non-BEAM languages. Since in those languages people typically don’t have lightweight processes, they usually resort to running multiple OS processes to split runtime activities. So for example, to start an external process registry, we would have a registry as an external OS process (e.g. etcd). To start/stop it, we would use some CLI API, probably in combination with an external lifecycle manager, such as systemd. Then we’d use some internal API (e.g. REST, possibly wrapped with some library functions) to interact with the running component. Such approach is in my view close to what you seem to be suggesting.
After using that approach myself for many years, and then also using the “BEAM way” for quite some time too, I empirically came to conclusion that I prefer the latter. I find it simpler, and at the same time more powerful and flexible. YMMV of course
In addition, it’s worth remembering that child_spec/1 thing is Elixir specific, and it’s in fact a fairly new addition to Elixir. FWIW, I think it’s one of the more interesting changes introduced by Elixir, because it makes things closer to the way I personally think about code. With child_spec & support by Elixir Supervisor, a component/service can fully encapsulate the details of how its started, which makes things simpler for the user. Stick Foo or {Foo, arg} where you want it in the supervision tree, and you can now interact with Foo using the functions from the same module (or “submodules”). To me that seems very straightforward. Again, YMMV
Well, you said earlier in this thread that it’s important that we’re discussing it, so here I am, discussing it
4