Is there a recommended approach or rule of thumb to determine how specific you should get when pattern matching a function’s arguments or is it purely preference?
For example, let’s say the function body only need the user’s id. Let’s assume that there are not multiple function heads. Which of these would you go with? Would any particular approach have benefits for the compiler / type inference?
def update(%Scope{} = scope) do
# use scope.user.id
end
def update(%Scope{user: user}) do
# use user.id
end
def update(%Scope{user: %User{} = user}) do
# use user.id
end
def update(%Scope{user: %User{id: user_id}}) do
# use user_id
end
Now let’s say your function needs multiple things from the %Scope, e.g. user_id, org_id, and something_else, would it change your approach?
The anti-pattern docs have a nice section about this. Personally I choose between these almost entirely on intuition - it’s a style thing. Just try not to overdo it. If the function signature gets to the point where you’re tempted to split it across multiple lines that’s a good hint you’ve gone too far.
I am to this day not especially impressed by the so-called anti-patterns in the official docs and was against a good chunk of them being introduced but alas, people started treating them as a gospel – which was the main argument against them in the first place. Official (anti-)patterns seem to shut down critical thinking often.
My rule of thumb with the years evolved to: use guards to assert data types, use the pattern-matching itself to assert on data shape. This is not exactly the same as pointed in the official docs; I like their idea of not mixing data extraction (deconstruction) and pattern-matching and I tried it but ultimately decided against it as it did not help me scan function heads quicker. I prefer to use guards for types and pattern-matching for deconstruction. Data extraction (deconstruction) being mixed with pattern-matching I don’t find a code smell.
As always, your mileage may vary. Just go for what makes the code more intuitive for you and your team. There are no strictly right or wrong answers.
I agree, I have been completely silent around this, since I did not see any arguments based on hard science, I don’t have an issue with soft science, but it becomes easily opinionated.
Why couldn’t they mainly be derived from physical constraints of the language, e.g how structs can lead to transitive compilation and there will never be a solution to this.
The thing is some people want official guidance on style (case in point, this thread), so it’s nice to have something to link them to.
Personally, like I said, I choose my style on intuition. The code for me is like a painting; I want it to look how I like. It’s an artistic endeavor. To this day I have never once run a code formatter, and I never will
Well, not like I was screaming about it either, just wanted to make it clear back then that it’s a slippery slope and it’s not a hard science indeed. I and others were told that this is by design and that the team does not believe they will be perceived as hard rules as much as we think. I disagreed but ultimately figured that critical thinking should lead the way for everyone and if not, well, they’ll be paying their own tech debt in the future, and bailed out.
Probably because you are not getting a compiler warning – or any kind of compile-time linting – that warns you about it. I just got reminded of it by you. Which is even more alarming because I actually knew it but forgot it. I think this should be a Credo check…
I would agree with you if not for the Git hosting services’ visual side-by-side diffs. I view my letting go of my own visual style of code and abandoning it forever as a personal character development in the name of less friction in collaboration. Less noisy diffs = win.
I somewhat envy you that you don’t have to abide by that.
Yep. In fact, when I read things like the “code smell” docs, I sometimes wonder if the first rule of such a document ought not be:
“over-compliance with these rules is itself a code smell because unquestioning devotion to our recommendations can be just as arbitrary and problematic as simply not being aware of the common pitfalls we discuss.”
As much as I’m untroubled by the phrases “code smell” and “anti-pattern”, we should recognize that they have an unwarranted absolutist quality to them. I do think many of the anti-patterns documented aren’t unreasonable as recommendations, but that many of them have contextually valid use cases and should not be avoided because some doc says it’s necessarily bad.
I recently very intentionally broke this rule, and did so with understanding of its caution. The reality in my case was that every way to re-write the the code to be compliant with the documented advice added complexity or avoidable mental context switching to follow the resulting code. In the end I didn’t feel bad about doing. However, could someone else knowing the “anti-pattern” doc simply look at what I did and hold their nose in disgust with no further consideration?.. I suppose so, but there should be limits on how far you indulge this tendency.
Anyway, to the point of the original question:
In function heads I tend to only use patterns for selecting the correct function head itself or to support guards. If I capture a value in function head pattern matching useful in the function: I absolutely use it later on, but I do try to deconstruct the data inside the function rather than in the function head. I think this makes it clearer what is required for function head selectivity vs simply trying to extract values from a complex data object inside the function. When I’m clearly addressing extraction, I don’t really hesitate to use pattern matches and even fairly complex pattern matches if the data warrants. So my advice is don’t mix purposes unless doing so makes busy work or things unnecessarily complex.
def drive(%User{age: age} = user) when age >= 18 do
%User{name: name} = user
"#{name} can drive"
end
def drive(%User{age: age} = user) when age < 18 do
%User{name: name} = user
"#{name} cannot drive"
end
Is there an anti-pattern for unnecessary decomposition? Both examples can do with user.name and user.age and then this second example would look much better (comparable to the first). Now the ‘correct way’ seems to add a lot of boilerplate and less readable code
Back the question: it depends. I usually mix when I need only 1 or 2 extra bindings and only one head. When multiple heads are involved I do try to follow the ‘only match’ pattern. It seems the docs nudge into that direction too.
The most readable code is the best code. What the best readable code is, depends on the context. Many unfamiliar devs in the code base that don’t know the domain well (oss libs)? Then extemely_clear_variable_and_function_names and more intermediate_variables_with_such_names. However, these names and intermediate vars can be distracting when not needed because all devs know the domain (work)
Also since the anti-patterns is opinionated it would be great to let the community regularly vote for each anti pattern inclusion or exclusion since style tend to evolve over time.
This is as close to “scientific” as it gets for me and what I generally follow myself. I also subscribe to the “do whatever is the most readable” approach.
Along these lines, multiple function heads with large amounts of destructuring also make error messages worse as it’s a pain to figure out which head it failed on.
That was the one and only argument against what I was doing in the past i.e. several big function heads. I saw it first-hand how doing the pattern-matching inside the body gives a more informative error that you can act on. That changed my mind.
Mind you, it was years ago and I was a bit stubborn back then. Sometimes I still prefer big function heads though, mostly for internal APIs where I know that no invalid data shape will hit the functions. But even under these conditions I reach for that technique more and more rarely.
Can’t answer what’s best, but I personally use pattern matching for flow control only. For destructuring I either use a library I wrote or simply access the structure fields directly.
When I do pattern match, I only pattern match against the part of the term I base the flow control on and nothing else, e.g.:
def update( %{ user: %{ id: nil}}) do
# the id == nil case
end
def update( %{ user: %{ id: _}}) do
# the case with id not being nil
end
As you can see, I don’t specify the module name (because I am not matching against different module names in this example) and I repeat the same path in both cases. This is to emphasize what I am truly pattern matching against with the least possible visual noise and to avoid defaulting to a wrong case if a different term is provided relative to what I’m interested in.
My personal rule of thumb, not unlike some of the views already expressed, is this:
Pattern match in the function head if that describes the requirements for that function to be called. This produces a mix of documentation and easy to read control flow which makes reading and understanding intent in the code easier. Knowing that a function requires a User struct, or it can operate on just any old map, for instance, is useful information both for me as a developer and the program.
Use guards for the same if it can not be (cleanly) accomplished with pattern matching in the function head
Pattern match on “top-level” data types that are themselves collections and which are vital variables for that class of function. This avoids repetitive code in the function body, and to signal that this function operates on that data. A good example of this is the assigns member of the socket parameter in LiveView handle_event functions. Calling socket.assigns is just noise in the function body, as is pattern matching it as the first line in the function.
Otherwise, I don’t pattern match in function heads more than that, and will deconstruct other data within the function themselves. Basically: if it isn’t control-flow, then it belongs in the function body, with the one exception described above.
What I have found over the years is that this allows me to scan function heads to understand the gross-detail control flow, while keeping function bodies a bit cleaner.
Well, it depends on how you want to express yourself vs. where it is that you want your code to fail (in case of an invalid argument).
If you’re only checking against nil because it’s an expected (valid) alternative AND you expect the function to fail later anyway if id is neither nil nor an integer then the code does not need an additional type guard.
Actually, you may just as well opt for an assertion like true = is_integer( id) within the function body to avoid “littering” the header (i.e. avoid reducing readability of the pattern matching intent in the function header).
As said before, it depends on the taste, but personally I dislike mixing flow control and argument validation.
