Hi,
I was surprised I couldn’t find any argument on this on the net so here I am.
When you make recursive function that deal with list and want to pattern match argument against [] or [head|tail] you can:
function [], do: *something*
function [h|t], do: *something else*
which is what I would call the Elixir way. A more haskellish way might be:
function argument do
case argument []
[] -> something()
[h|t] -> something_else()
end
end
The first option allow you to handle different arities the same way but is there other good arguments on why you should use the first option instead of the second ?
The first combo is mostly used to iterate over a list and have the [] mark completion.
The second while being correct, probably feels less idiomatic and more difficult to interpret.
def process([]),do: :done
def process([item | rest]) do
.... do something and then
process(rest)
end
versus
def process(list) do
case list do
[] -> :done
[item | rest ] ->
.... do something and then
process(rest)
end
end
It is a matter of preference.
Also in the second version you are tempted to add more code after the case statement and I am not sure if this interferes with tail-end-recursion optimisation. I’d opt for option 1 to be sure.
I’ve been in a team where they insisted on option number 2. Their reasons were that if we get unexpected input to the functions we will not get FunctionClauseError in our monitoring system; we’ll get MatchError instead. They argued this is more intuitive and immediately gives you a clue what’s wrong.
I disagreed with them but after several discussions (that, thinking back, I really should not have insisted on; if the CTO says “no” it’s often best to just move on with life but hey) I agreed to let it go.
I am siding with @Hermanverschooten here: this is basically a matter of preference.
Pattern matching in the function head should be used for flow control.
Pattern matching in the body of the function should be “this function requires this data”
It’s a subtle but important distinction.
Yes MatchError is better than FunctionClauseError, especially in the case where you have multiple function heads. It is much easier to jump to the line number of the MatchError than it is to figure out which function head is missing an assign in a large assigns crash dump.
Elixir will only show the line number of the first defined function head (on a FunctionClauseError). It will also log each function head it attempted to take - if each function head is unwrapping several values that are orthogonal to the flow control, it becomes difficult to tell which one should have been taken.
That leaves the developer in the unfortunate position of figuring out
Which function head should have been taken with this call?
Which missing value caused this function invocation to fail?
In my context, I unwrap only what’s necessary to determine which function head to take:
def complete_chat(%{ai_service: :open_ai} = attrs) do
%{messages: messages} = attrs
# call openai api here
end
def complete_chat(%{ai_service: :google_bard} = attrs) do
%{messages: messages} = attrs
# call google bard here
end
MatchErrors now point me directly to the specific function head where the error occurred
Eh, yes and no. If function heads grow that big so as troubleshooting becomes difficult, this outlines two serious problems:
Why do we get FunctionClauseError at this non-early stage of the project still? Those bugs should have been fixed already. They are early-stage “we are still not sure what this external API might return” things. After one month in production these should be gone.
When the sizes are manageable it’s very easy to tell which function head was attempted, especially having in mind that Elixir and the monitoring system will also show you the function arguments that were attempted.
So you’re not wrong on the outset but I’d argue that the issues you mention absolutely should be non-issues at this lifecycle stage of the project.
To address your next comment: sure, that’s valid. I suppose for part of the scenarios that technique can help. I still would not though, to me function head pattern matching is an all-or-nothing. When we start mixing both that + matching in body it gets confusing and code-smelly to me. I’d rework it.
Yeah, same thinking. If there’s anything I can relegate to the computer to fail early so I can fix it quickly and move on, I’ll do it. I’ll take this to the absolute extremes. (To the point that I worked with comby and semgrep several times to try and detect anti-patterns in code; hey, if I succeed then maybe that’s one thing I can charge money for consulting, who knows!)
Rant blurb: I really feel people often forget “computers must serve us, not us serving them”. (And this is not pointed at the other poster @JohnnyCurran, just a general complaint.)
I understand and I do empathize. I often find myself wanting to unwrap everything in the function head. I’ve found, however, after over 4 years of production elixir, that it causes more problems than it solves
in particular, how do you square:
function head pattern matching is an all-or-nothing
with:
When the sizes are manageable it’s very easy to tell which function head was attempted
At a certain point, especially on a team, the sizes of parameters are going to get large. It’s inevitable. is it all-or-nothing, or is it a manageable size?
And, to be honest I’m not sure I totally understand this question:
Why do we get FunctionClauseError at this non-early stage of the project still
If you are suggesting we all write error-free code, I agree, but I don’t know how practical that is. FunctionClauseErrors aren’t exclusive to third-party apis, and, like everything in software – things change. To suggest “just don’t write code that throws a FunctionClauseError” seems misled
In any case – I have genuinely enjoyed this discussion with you as I love discussing the ins-and-outs of Elixir with whoever is unfortunate enough to be within earshot , especially so with those whom which I disagree. There is no growth without disagreement. All that to say that I hope my tone has not come off as disrespectful which I know it has the possibility to do over text, especially over the wider internet
Nah, not saying that, I am saying that FunctionClauseError is a symptom of a very early stage problem. Hence, having everything be in the function header is a non-issue for me.
As for them growing big, yes, I can see that. But I would rework that into smaller functions e.g. in your example I would pass attrs to sub-functions.
Not at all, these are genuine ergonomics that can and have tripped people – myself included – so it’s worth debating especially if we end up in the same team.
I just want to mention that I think what you are “calling smelly” is actually the preferred refactoring for one of the anti-patterns in the Elixir documentation: Complex extractions in clauses. It might be worth thinking about it once more and weighing off the pros and cons. At least for me it was something that looked like a weird recommendation at first, but I started liking it. But maybe I misunderstood your point completely…
Edit: I just realized that’s pretty much what @JohnnyCurran already stated with
I am aware, thank you. We have the right to disagree with some of them.
As suggested above, I’d nest the deconstruction of a more complex piece of data, meaning I’d do a partial reconstruction at the top-level function(s) and then pass parts of it to nested ones where they also reconstruct. Huge deconstruction heads indeed are not easy to visually parse sometimes.
It makes no difference at all with regards to efficiency. Literally the complier translates it into almost the same code with the main difference being what error you will get.