Elixir for Programmers course - should we adopt Dave Thomas' way of organizing GenServers?

Yes, I like this layout. I haven’t done/seen the course though so I am speaking without the full context.

This is the only thing I am questioning. From my experience the layout Dave suggests is a common way to layout code in erlang.

My standard layout in erlang (which I have seen in multiple erlang projects as well, I probably took it or was inspired by Erlang and OTP in action) is:

• src/myapp.erl - Includes the API. For smaller projects this is the only module used and all others are internal implementation detail. All access to the application is done though here.
• src/myapp_some_server.erl - Gen server implementation. Should usually be thin and only handle messages.
• src/myapp_whatever.erl - To support business logic and other things. These modules are generally referential transparent to help with testing, robustness. Often doesn’t have any API guarantees between versions as they are internal to the application.

Yes, but as with everything there is a trade-off. With large and complex applications even following the single responsibility principle will give you large modules which may be hard to grok at a quick glance and require plenty of domain knowledge to understand.

I’m new to the Elixir world and just finished @pragdave’s course. But I really like the way he laid out the structure and in my limited Elixir experience, his way just seems to make a lot of sense.

Mostly because it separates the concerns and think it would make testing easier.

So no, you are not the only one

Robert Virding mailed sometime ago on this list that he is not a fan of Dave Thomas’ idea of splitting API functions and server functions. See Elixir Blog Posts - #241 by nucleartide. I for me am not convinced by Thomas’ reasoning, I don’t find his idea interesting. I still want to look at GitHub - sasa1977/exactor: Helpers for simpler implementation of GenServer based processes · GitHub to simplify genserver implementations.

I’ll play a devil’s advocate here for a little.

What is the GenServer’s callback module other than the raw “business logic” module already? It’s already an isolated implementation that extracts the common logic from what is specific to your app. I’m not sure adding another layer in-between is actually helping with anything.

I think what Dave was demonstrating in his course was not the “best way” to organize code, but the process and logic behind refactoring in Elixir. It’s interesting to compare Lance Halvorsen’s approach to factoring out business rules in his book Functional Web Development with Elixir, OTP and Phoenix. They both do a great job of breaking business rules out in their API designs but with slightly different naming conventions. In both cases they end up with well organized applications that have clear separation of concerns.

I believe that there are two different discussions here:

1. Should “business logic” (i.e. state management) reside in a separate module?
2. Should GenServer API be placed in a separate module?

When it comes to first question, you and Dave are certainly not the only ones thinking that way. You’ll find the same line of thoughts in my writings in Elixir in Action and To spawn or not to spawn. I believe that the same train of thought is also present in Lance’s Functional Web Development.

Besides just public writing, I tend to use this style in “real life” projects. However, I do not go for it 100%. If the state is fairly simple, I just keep it inside the GenServer module. When it becomes more complex, I extract it.

In some situations, I decide upfront that the state handling is complex, and so I decide to extract it upfront. In fact, in such situations, I immediately start working on the state, and not on the GenServer. A bit unusual example is my Parent library, where I first wrote the state logic, then I added a procdict wrapper, and only then did I wrap it inside a GenServer.

There are also some cases where I extract the state management into a separate module, but I keep some orthogonal parts, such as handling timer messages in the GenServer.

But as a rule, I definitely think that state management mostly belongs outside of the GenServer callback module. This leads to a nice separation of concerns, improves code reading experience, supports testability, and promotes code reusability.

OTOH, I’m mostly not convinced that API should be separated from the server side call (GenServer callbacks). I believe the two are naturally coupled, and thus usually belong together.

I can see some possible situations where this wouldn’t hold, for example if there are lot of complex transformations in interface functions, or if you want to vary the server implementation but use the same interface (basically a process-based polymorphism). In my opinion such cases are not very common, and so I think that prematurely extracting API into a separate module will mostly leads to unnecessary level of indirection and reduces the reading experience. It sort of reminds me of the OO style where each member variable in a class is immediately wrapped behind getter/setter functions.

So while I agree that there are some situations where that separation would be beneficial, I think that a good default is to keep API close to the server side handles (because after all they are naturally coupled), and extract if there’s particular need.

I love a few ideas from his course, he is obviously a very experienced guy who has worked in a ton of languages. However, there is one thing I find a bit unfunctional and a bit more OOPy using of the @me constant to refer to the same module:

defmodule Tasks.Sync do

  @me __MODULE__
  def start_link(args) do
    GenServer.start_link(@me, args, name: @me)
  end
end

I prefer adding an alias and using the Module’s name instead of a module attribute:

defmodule Tasks.Sync do

  alias __MODULE__

  def start_link(args) do
    GenServer.start_link(Sync, args, name: Sync)
  end
end

As @sasajuric says there are really two issues here: where to put the API; and where to put the “business logic”.

For the first question I think that they should definitely both be in the same module as they are totally integrated with each other so splitting them up doesn’t feel right. And there is honestly not much code in the client side.

The second question is more complex. In my mind there are a number of factors on which this depends. I think it is easiest if I (briefly) present how I structure my gen_server modules. So they all basically follow the same structure and ordering of parts:

First comes the “management API” section by which I mean the calls to start/stop the server: start, start_link and stop.

Then the “user/client API” section which is the calls users/clients make to access the server. These are the ones calling GenServer.call/cast.

Then come the section with all the callback functions but in a specific order:

• init/1 and terminate/2
• handle_call/3, handle_cast/2 and handle_info/2
• code_change/3

I keep this order as it makes easier to find the functions. Also I generally include all the callbacks even if they are not used as I can directly see what each one does and there is no chance that I miss one and get it wrong. Being explicit rules in the long run (like next week ) and the extra code is negligible, seriously it is. I also generally do very little inside the actual callback functions themselves but call local functions to do most of the work, unless what is done is trivial. This make reading the callback functions easier, especially when there are many clauses. I find this more important with elixir than erlang as the elixir syntax tends to hide that we are really talking about multiple clauses of the same function and not multiple functions.

Finally come the section with all the local functions which generally implement most of the logic. I never ever mix the callbacks with the local functions as, again, keeping them separate makes it easier to find things.

This organisation means that breaking out the business logic into a separate module becomes less important as it is already separated into its own section. Again whether to break it out or not depends on a number of things, for example how much code there is. Also if you do break out the code into a separate module then you must move all the logic to this module so you don’t split it into different modules which tends to be a bad thing. I am assuming that this logic is just used in the gen_server and not in other places, if not then it should definitely be moved into a separate module as you only want to call the gen_server module for accessing the server.

So my thoughts on the matter. It became a bit longer than I had intended.

Final note here: I think that being explicit is extremely important in the long run. The longer your code is used the more being explicit helps, especially if it gets to the stage where someone apart from you has to manage the code. And seriously, it is generally not much code we are talking about.

The advantage of Dave’s method, I find, is that you now have one module you can open (either its source code or its documentation) which announces “I’m the public API, everything I have can be touched” - so even if it maybe makes it harder for the author to organize things, it makes things much simpler for the reader, and in my opinion, code is meant to be written for the person reading it (including the proverbial you, one year down the road, in the middle of the night, while stuff is down).

I do agree that having three modules for a GenServer that maybe just one or two things on top of what an Agent would to is a think you can discuss - design decisions are never black and white. The point I’m trying to make is that, as a community, we’re pushing (by making it the default and using it in code everywhere) a pattern of “it’s ok to have API, GenServer glue, and logic in a simple module” and I think that Dave makes a point that that pattern is more wrong than right and we maybe should stop advertising it.

I’m not sure I follow this point. It seems that you refer to the fact that with the API module you can see the interface of the module, and nothing else. If that’s the point, I personally don’t find it very relevant, but it admittedly depends on the editor you’re using and the code culture.

At my company, we use @impl for all the callbacks, which implicitly includes @doc false, and so a callback function is not considered to be a public API, and won’t be included in generated docs. Moreover, I’m using VSCode with ElixirLS, and so I get a proper member list which includes only API functions.

Finally, we organize our code to place API functions into a separate section at the top of the file, which means that even without the editor support it’s fairly easy to quickly scan the file and see all of the API functions.