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.

Ok. I found most of the servers that I could distribute are pretty small but this one covers most of the ground. It is in Erlang which I mostly use but it should illustrate what I mean. The code is the simulation master from a spaceship simulation where the ships are implemented in Lua using my Luerl package.

-module(sim_master).

-behaviour(gen_server).

-define(SERVER, sim_master).
-define(TABLE, sim_ship_array).

%% User API.
-export([start/3,start_link/3,stop/1]).
-export([start_run/1,start_run/2,stop_run/0,stop_run/1]).
-export([get_ship/1,get_ship/2]).

%% Behaviour callbacks.
-export([init/1,terminate/2,handle_call/3,handle_cast/2,
         handle_info/2,code_change/3]).

%% Test functions.
-export([init_lua/0,load/3]).

-record(st, {xsize,ysize,n,arr,tick=infinity,st}).

%% Management API.

start(Xsize, Ysize, N) ->
    gen_server:start({local,?SERVER}, ?MODULE, {Xsize,Ysize,N}, []).

start_link(Xsize, Ysize, N) ->
    gen_server:start_link({local,?SERVER}, ?MODULE, {Xsize,Ysize,N}, []).

stop(Pid) ->
    gen_server:call(Pid, stop).

%% User API.

start_run(Tick) ->
    gen_server:call(?SERVER, {start_run,Tick}).

start_run(Sim, Tick) ->
    gen_server:call(Sim, {start_run,Tick}).

stop_run() ->
    gen_server:call(?SERVER, stop_run).

stop_run(Sim) ->
    gen_server:call(Sim, stop_run).

get_ship(I) ->
    gen_server:call(?SERVER, {get_ship,I}).

get_ship(Sim, I) ->
    gen_server:call(Sim, {get_ship,I}).

%% Behaviour callbacks.

init({Xsize,Ysize,N}) ->
    process_flag(trap_exit, true),
    {ok,_} = esdl_server:start_link(Xsize, Ysize),
    {ok,_} = sim_renderer:start_link(Xsize, Ysize),
    {ok,_} = sim_sound:start_link(),
    {ok,_} = universe:start_link(Xsize, Ysize), %Start the universe
    random:seed(now()),                         %Seed the RNG
    Arr = ets:new(?TABLE, [named_table,protected]),
    St = init_lua(),                            %Get the Lua state
    lists:foreach(fun (I) ->
                          {ok,S} = start_ship(I, Xsize, Ysize, St),
                          ets:insert(Arr, {I,S})
                  end, lists:seq(1, N)),
    {ok,#st{xsize=Xsize,ysize=Ysize,n=N,arr=Arr,st=St}}.

terminate(_, #st{}) -> ok.

handle_call({start_run,Tick}, _, #st{arr=Arr}=St) ->
    %% We don't need the Acc here, but there is no foreach.
    Start = fun ({_,S}, Acc) -> ship:set_tick(S, Tick), Acc end,
    ets:foldl(Start, ok, Arr),
    {reply,ok,St#st{tick=Tick}};
handle_call(stop_run, _, #st{arr=Arr}=St) ->
    %% We don't need the Acc here, but there is no foreach.
    Stop = fun ({_,S}, Acc) -> ship:set_tick(S, infinity), Acc end,
    ets:foldl(Stop, ok, Arr),
    {reply,ok,St#st{tick=infinity}};
handle_call({get_ship,I}, _, #st{arr=Arr}=St) ->
    case ets:lookup(Arr, I) of
        [] -> {reply,error,St};
        [{I,S}] -> {reply,{ok,S},St}
    end;
handle_call(stop, _, St) ->
    %% Do everything in terminate.
    {stop,normal,ok,St}.

handle_info({'EXIT',S,E}, #st{arr=Arr}=St) ->
    io:format("~p died: ~p\n", [S,E]),
    ets:match_delete(Arr, {'_',S}),             %Remove the ship
    {noreply,St};
handle_info(_, St) -> {noreply,St}.

%% Unused callbacks.
handle_cast(_, St) -> {noreply,St}.

code_change(_, St, _) -> {ok,St}.

%% Local functions.

%% init_lua() -> LuaState.
%%  Initialise a LuaState to be used for each ship process.

init_lua() ->
    L0 = luerl:init(),
    L1 = lists:foldl(fun({Name,Mod}, L) -> load([Name], Mod, L) end, L0,
                     [{esdl_server,luerl_esdl_server},
                      {universe,luerl_universe},
                      {sound,luerl_sound},
                      {ship,luerl_ship}]),
    %% Set the default ship.
    {_,L2} = luerl:do("this_ship = require 'default_ship'", L1),
    L2.

load(Key, Module, St0) ->
    {Lk,St1} = luerl:encode_list(Key, St0),
    {T,St2} = Module:install(St1),
    luerl:set_table1(Lk, T, St2).

start_ship(I, Xsize, Ysize, St) ->
    %% Spread out the ships over the whole space.
    X = random:uniform(Xsize) - 1,
    Y = random:uniform(Ysize) - 1,
    {ok,S} = ship:start_link(X, Y, St),
    %% Random speeds from -0.25 to 0.25 sectors per tick (very fast).
    Dx = 2.5*random:uniform() - 1.25,
    Dy = 2.5*random:uniform() - 1.25,
    ship:set_speed(S, Dx, Dy),
    {ok,S}.

Yes, it is pretty comment-free but it is not for release yet.

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.