In my opinion, over use of then is a code smell; your maybe_do function is not.
derek-zhou
sodapopcan
It’s very hard to tell without seeing your exact code, but if you’re feeling it’s a code smell it’s possible that you are trying to force a pipeline when it is not needed. Once you start threading tuples like {event, socket} through a pipeline where only one function cares about event, you’re hiding details. It makes it harder in the future to read the pipeline at a glance without looking at the implementation of each function. Of course if it’s a pattern that appears everywhere it’s not such a big deal, but generally pipelines are best when they operate on a single immutable data structure. YMMV, of course.
5
Gilou06
aziz
For this I have a little macro in my toolbox:
defmacro then?(value, pred, fun) do
quote do
if unquote(pred) do
unquote(fun).(unquote(value))
else
unquote(value)
end
end
end
To me it’s a good complement to the existing then/2 macro. I tried to get it added 2 years ago, but no luck. 
3
ityonemo
I usually do this:
def if(value, conditional, _) when conditional in [nil, false], do: value
def if(value, _, fun), do: fun.(value)
0
dimitarvp
Fair, but I don’t believe in “truthy” and “falsy” values. If it’s not a boolean then it’s an invalid input.
2
ityonemo
You are free to take that attitude, but you are leaving powerful functions for communicating intent like List.wrap/1 off the table.
0
derek-zhou
I have something even eviler:
defmacro value ~> name do
quote do
value = unquote(value)
unquote(name) = if value, do: value, else: unquote(name)
end
end
defmacro name <~ value do
quote do
value = unquote(value)
unquote(name) = if value, do: value, else: unquote(name)
end
end
So I can do:
socket
|> step1()
|> step2()
~> socket
socket <~ if(condition?, do: step3(socket))
socket
|> step4()
|> step5()
1
dimitarvp
How so? I am curious. Could you give an example?
I cringe when I see stuff like if list ... when list should never be nil anyway. If a list might be nil (f.ex. if you can’t control it when a framework is calling your callback or such) then it’s best if you do call_function(List.wrap(list_or_nil), ...).
And I believe this is not just some random personal preference. I prefer code to communicate its intent clearly. Just doing if this_could_be_anything ... with no regard of its type is to me a code smell because it’s not clear what the intent is. Might as well write PHP or JS at that point. We should use all of Elixir’s strengths.
Furthermore, I’d prefer to strictly assert when is_list(parameter) in my function signatures as well (or use the [first | rest] pattern-matching idiom, optionally together with [] if an empty list is an acceptable parameter value).
0
dimitarvp
To elaborate with an example of my own, this I consider noisy code that over-compensates for something we can avoid:
defmodule Accept.List do
def work(nil), do: {:error, :invalid}
def work([]), do: {:ok, :nothing_to_do}
def work([first | rest]), do: {:ok, :we_have_done_stuff}
end
This introduces two problems:
- We have to think of a return value (or a logging event, APM notification etc.) for when stuff is
nilwhen it should not benil. Mind you, this might be desirable for some projects and I’ve been part of them. When this is something you are interested in then yes the above code is good. Has been a rarity in my practice but I recognize that it does happen. - We pollute this (and likely other) module with
nilclauses for functions when it’s very likely we can protect againstnil(or convert it to a default empty value e.g.[]for lists) at the call-site e.g.list_or_nil |> List.wrap() |> Accept.List.work().
Whereas, if we remove the nil clause above we’ll make our caller blow up with an error, thus making it very clear that we’ve made a mistake (vs. logging stuff or putting a non-fatal message in an APM system might not draw the programmer’s attention). Whether that mistake can be worked around with a default value or that’s truly an error depends on the project but I’d still prefer to “hear” about an invalid input to my function(s).
Of course this is all bike-shedding, more or less. I’m still defaulting to things blowing up vs. doing too much defensive programming and ending up with code that has no idea what to do with the hot potato when it lands on its lap.
1