rogach
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?
Where Next?
Trending in Questions
Other Trending Topics
Latest on Elixir Forum
Sponsor Spotlight
Error tracking, logs, APM, uptime and cron monitoring in one dev-friendly platform. Find and fix problems before your users notice.
Categories:
Sub Categories:
Forums
Popular Tags
- #ecto
- #liveview
- #troubleshooting
- #learning-elixir
- #library
- #deployment
- #erlang
- #testing
- #genserver
- #mix
- #absinthe
- #remote-other
- #otp
- #plug
- #how-to-question
- #macros
- #postgres
- #elixirconf
- #channels
- #exunit
- #discussion
- #code-sync
- #podcasts
- #javascript
- #onsite
- #dialyzer
- #docker
- #authentication
- #umbrella
- #full-time-contract
- #podcasts-by-brainlid
- #ai
- #ecto-query
- #elixirconf-us
- #blog-post
- #elixir-ls
- #phoenix_html
- #iex
- #graphql
- #genstage
- #websockets
- #supervisor
- #advent-of-code
- #distillery
- #processes
- #elixirconf-eu
- #api
- #forms
- #metaprogramming
- #hex
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.
Error tracking, logs, uptime, and Just Enough APM. Built for developers.
Producing high quality Elixir screencasts since 2017.
**Building the future of software with AI, trust, and human connection.**
The deployment platform built for Elixir: PaaS ease, VPS control.
The team behind Membrane, Popcorn, LiveDebugger. Available for hire.
Courses that'll move you from confusion to "Aha, now I get it!"
Showing Posts 1 to 10- Show Best Posts
- Show All (oldest first)
- Show All (newest first)
LostKobrakai
It’s a little unclear why
LineReaderwould need access toparent_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 theClientHandler. Yes this might be a “bit more characters” inClientHandler, but it’ll be less inLineReaderand responsibilities are properly aligned.rogach
That’s probably the part I’m most unsure about. I’ve chosen that approach mostly because of readability, and in addition because of less possibility for an error.
Compare:
with:
While second version doesn’t need to access parent state, it adds quite a bit of ceremony and makes it easy to actually forget to update parent state field (immutable nature of Elixir might make it very easy to forget the second line and result in a bug).
rogach
I’m incorrect here - Elixir compiler will warn about an unused lr_state value, so it won’t be that easy to forget to update the parent state.
LostKobrakai
You could look into
get_and_update_in(),update_in, ….rogach
I don’t see how
update_inorget_and_update_inwill be useful here - we actually need to get thelinesdata out, andupdate_inonly allows us to mutate contents of the map without returning anything.LostKobrakai
get_and_update_incan return you a value and the update part could just be a noop if necessary.rogach
I see now. So something like that will work:
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.
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
These might be signs that the
LineReaderwould be better off as its own process i.e. Task/Agent/Genserver so it can keep track of its own state or rolled intoClientHandler. I’d be curious what else it’s being used for and how else it’s being used.