rogach
Is mutating "parent" state in a module an anti-pattern?
I often need to abstract some common functionality into a separate module. In cases when this functionality requires keeping some state, I use the following pattern: I keep that state under a separate key in the “parent” module state (usually a GenServer), and to simplify the code I pass “parent” state into the functions of common module.
I’ll give a specific example to make the idea clearer. In the example, there is a GenServer module that handles packets coming from a TCP socket, and there is LineReader module that handles aggregating the raw data packets and extracting lines from them.
Here’s the code (I’ve attempted to omit most of irrelevant parts):
defmodule ClientHandler do
use GenServer
def init(%{socket: socket}) do
state = %{
socket: socket,
lr_state: LineReader.initial_state(),
}
{:ok, state}
end
def handle_info({:tcp, _socket, data}, state) do
state = LineReader.put_packet(state, data)
{lines, state} = LineReader.recv_lines(state)
# ... do something with lines
{:noreply, state}
end
end
defmodule LineReader do
def initial_state() do
%{lines: [], trailing: ""}
end
def put_packet(parent_state, packet) do
lr_state = parent_state.lr_state
# ... process the new data packet, update lr_state
%{parent_state | lr_state: lr_state}
end
def recv_lines(parent_state) do
lr_state = parent_state.lr_state
lines = lr_state.lines
lr_state = %{lr_state | lines: []}
{lines, %{parent_state | lr_state: lr_state}}
end
end
I feel like this code is a little smelly, because it ties into specific field name in parent state. However, it saves a lot of code on the caller’s side and makes the code much cleaner.
Is such approach an anti-pattern? Are there some better alternatives?
Most Liked
LostKobrakai
Imo processes should only be introduced if there are runtime level reasons for them - failure isolation, asynchronous processing, … That is completely separate from code level abstractions, where multiple module collaborate to build up logic.
dimitarvp
A super generic answer: be explicit. Implicitness is rarely worth it unless it’s seriously getting in the way. One example of “getting in the way”, subjectively for me, is a lot of artifacts that Phoenix generates should start off being referenced as defaults in other modules / libraries and only put inside your code base when you need to modify them and they are no longer the defaults.
In your case however, it does seem you want to save just a little typing which is IMO not worth.
And this is also very related to feature envy and the single responsibility principle. Simplified: put things where they semantically belong. Which of course is the entire problem, sometimes we have a problem nailing the semantics – and is ironically one of the things we are actually being paid to do, not the code flinging itself.
LostKobrakai
It’s a little unclear why LineReader would need access to parent_state. I’d argue that LineReader should only be involved with the state it takes care of and not care that it’s nested within the ClientHandler. Yes this might be a “bit more characters” in ClientHandler, but it’ll be less in LineReader and responsibilities are properly aligned.
derek-zhou
Smelly code is not clean code. My advise is to make the code not smelly first. We can then help you to refactor the code to be more concise and readable.
codeanpeace
Yeah, agreed – it’s what I was trying to suss out with “what else it’s being used for and how else it’s being used”. It’d be the code organization by process anti-pattern otherwise.
You can spin up a temporary process “on demand” e.g. Task.async/1 and “if you spawn a task inside a GenServer, then the GenServer will automatically await for you and call GenServer.handle_info/2 with the task response and associated :DOWN message.”
If the data packet processing logic is brittle and the client handler needs to be robust, a nice runtime reason could be failure isolation so that packet processing can fail without affecting the client handler. You’d likely want to only pass the necessary information.
Popular in Questions
Other popular topics
Latest on Elixir Forum
Sponsor Spotlight
Catch errors, track performance, monitor hosts and more.
Categories:
Sub Categories:
Forums
Our Sponsors
Build Elixir applications with speed and confidence.
Supporting innovation across the BEAM ecosystem.
We build reliable cloud platforms for business-critical systems.
Catch errors, track performance, monitor hosts and more.
Practical resources that improve the lives of professional developers.
Producing high quality Elixir screencasts since 2017.
Enabling companies to succeed by building software people love.
Courses that'll move you from confusion to "Aha, now I get it!"