quda
I’ve got this function, translated from an old codebase (imperative language) into Elixir, by our contractor.
def pub0(payload) when is_map(payload) do
if ["country", "commodity", "share", "mon", "cog", "esa1"] |> Enum.all?(fn x -> Map.has_key?(payload, x) end) &&
Map.get(payload, "esa1") == "hs10" &&
is_intger(Map.get(payload, "cog") do
payload = Map.put(payload, "itime", utimestamp(2))
emporio_ts(Jason.encode!(payload))
end
end
The function works fine, it performs its job, but I don’t like the code structure:
There is an if with tree conditions that have to evaluate all to true (logically: if (true & true & true) then …). If satisfices this, then adds new property to the object and finally calls another function.
It does not seem functional. Is there a way to refactory the function in a (more) elegant and functional manner? And also more comprehensible, consistent, shorter ?
Thanks! 
Where Next?
Trending in Questions
Other Trending Topics
Latest on Elixir Forum
Sponsor Spotlight
Enabling companies to succeed by building software people love.
Categories:
Sub Categories:
Forums
Popular Tags
- #ecto
- #liveview
- #troubleshooting
- #learning-elixir
- #deployment
- #library
- #erlang
- #testing
- #genserver
- #mix
- #absinthe
- #remote-other
- #otp
- #plug
- #how-to-question
- #macros
- #postgres
- #elixirconf
- #channels
- #exunit
- #discussion
- #code-sync
- #javascript
- #podcasts
- #onsite
- #dialyzer
- #docker
- #authentication
- #umbrella
- #full-time-contract
- #podcasts-by-brainlid
- #ecto-query
- #blog-post
- #elixir-ls
- #ai
- #elixirconf-us
- #phoenix_html
- #iex
- #graphql
- #genstage
- #websockets
- #supervisor
- #advent-of-code
- #distillery
- #processes
- #api
- #forms
- #hex
- #security
- #metaprogramming
Our Sponsors
Build Elixir applications with speed and confidence.
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.
Elixir, Erlang & BEAM conferences from Code Sync - join us in 2026!
Practical resources that improve the lives of professional developers.
Master modern tools, write cleaner code, and tackle real-world challenges.
Enabling companies to succeed by building software people love.
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 6- Show Best Posts
- Show All (oldest first)
- Show All (newest first)
trisolaran
Personally, I’d probably go for this:
al2o3cr
The usual micro-scale cleanups could help (extract the parts of that giant conditional into functions with meaningful names, for instance) but the shape-checking on
payloadmakes me wonder if it should be a struct.That would require much larger changes to the calling code, but simplify
pub0considerably.quda
This looks very cool, but it fails:
(MatchError) no match of right hand side value.Maybe because the payload has much more properties than those six tested for. The rest are not mandatory but in total are ~30 properties.
Actually I learnt that (in prod) the number of props of the payload is variable, between 20 and 35.
trisolaran
No, maps (thankfully) support partial matches
Share your code and the exact map you’re passing to
pub0, I’m sure we can figure out what’s going onquda
I’ve learnt map support partial matches but still it fails with that error.
Unfortunately, I can’t share more code/data (for legal reasons), I have to debug it myself.
cmo
It can’t fall with that catch-all returning
nil…?