Guard clause on match operator

[sodapopcan]

I was trying to get further involved but now you pinged me, d’oh

@dimitarvp

Guards “should not”

I never said “should.” OP asked why things are the way they are and I was trying to provide an answer. People are free to write code how they want. Also, OP is asking why we can’t do guards in inline matches. You example uses case.

In short, @zachallaun said what I meant but far more eloquently. By “not meant for type assertions” I meant that they are “meant” for flow control. And I don’t buy that they are 100% same thing, though of course they are very closely related. For example in LiveView, José recommends (not a “commands”) matching only against keys that will satisfy a match:

def handle_event("foo", %{"id" => id}, socket) do
  current_user = socket.assigns.current_user

as opposed to something like:

def handle_event("foo", %{"id" => id}, %{assigns: %{user: user}} = socket) do

@giddie

We must be talking at crossed purposes, because I’d say that OTP is pretty much the embodiment of defensive programming.

It’s already been touched on but your definition of defensive programming is not the “regular” one and it’s certainly not Erlang’s definition. Generally when we do inline matches it’s on stuff that can fail either because it’s a side effect or the result of parsing user input:

:ok = network_request()
{:ok, result} = parse_user_input_i_wont_deal_with_if_its_wrong_for_some_reason("hi!")

You otherwise don’t usually see random type assertions in a function body. I’m actually having a hard time picturing when this would even be useful—why wouldn’t you check it in the function head? In any event, if you have a use-case that’s fine but again I was just trying to answer the question. There is a direct example of how this is not intended in the link above but program however you want.

@giddie

@zachallaun

I also feel like invariants should be checked at boundaries, not scattered throughout the code.

Yeah, I definitely agree with this. But I’d also 100% prefer a pattern match error near some unexpected data over some obscure key :whatever not found in: nil waaay down in the stack.

This is sort of missing the point—if you’ve checked at the boundaries and you’re getting a nil error, your boundary-checking is wrong. The whole point of boundary casting is to get data into a a solid, known shape (ideally a struct AFAIC though whole other convo).

@giddie

I’d go so far as to say that guards semantically are pattern matches. They offer programmable extension to patterns.

Agreed. The docs even touch on this.

[dimitarvp]

@sodapopcan

I was trying to get further involved but now you pinged me, d’oh

Yeah sorry, thought I missed a nuance because what I think I got didn’t quite add up. Thanks for clarifying.

@sodapopcan

This is sort of missing the point—if you’ve checked at the boundaries and you’re getting a nil error, your boundary-checking is wrong. The whole point of boundary casting is to get data into a a solid, known shape (ideally a struct AFAIC though whole other convo).

It’s very possible to have an error in the middle of a bigger function to which my answer 99% of the time is “break it apart in smaller functions and still check at their boundaries”.

[giddie]

@sodapopcan

You otherwise don’t usually see random type assertions in a function body. I’m actually having a hard time picturing when this would even be useful—why wouldn’t you check it in the function head?

For example:

def my_func(opts \\ []) do
  {:ok, value} = Keyword.fetch(opts, :my_integer)
end

Do I really need to split out a separate function just to ensure my value is an integer? I think it would just be nice if I could add a guard clause here on the match operator to quickly make the assertion for peace of mind before moving on. And really, I’m curious about whether it was deliberately left out by design choice. My suspicion is that it was actually left out for reasons of legibility or because of ambiguity in the AST.

@sodapopcan

your definition of defensive programming is not the “regular” one

I get where you’re coming from. You could be right. What I mean is, one definition is:

A form of defensive design aiming to ensure the continuing functioning of a piece of software in spite of unforeseeable usage of it.
- Wikipedia

To me, this is a great description of OTP. And when I write code in Elixir, I aim to write code that makes use of this defensive design by asserting that the data is as expected, knowing that if it’s not the software as a whole will not crash. I consider that programming defensively - the more assertions, the more defensive. But yeah - looking just at my code, I’m not trying to handle all possible error conditions myself if that’s the way we’re defining it.

[dimitarvp]

@giddie

Do I really need to split out a separate function just to ensure my value is an integer?

Elixir is strongly but not statically typed so I’d say yes you should, and make use of the tools available to us f.ex.

{:ok, price} = KeywordUtil.fetch_integer(opts, :price)

# elsewhere:

defmodule KeywordUtil do
  # NOTE: `Keyword.fetch` already has guards for both arguments.
  def fetch_integer(kw, key) do
    case Keyword.fetch(kw, keys) do
      {:ok, int} when is_integer(int) -> {:ok, int}
      {:ok, val} -> :error
      :error -> :error
    end
  end
end

Seeing as this is your specific need then I see no reason why you wouldn’t make wrapping helpers.

…Or we should all just use NimbleOptions. Because apparently it’s very far from only you-specific need, I needed such wrappers a number of times.

@giddie

And really, I’m curious about whether it was deliberately left out by design choice.

IMO because it’s teetering on Elixir trying to imitate a statically typed language in such a case. If you allow guards on assignments you are like 50% of the way there to static typing.

[sodapopcan]

I don’t want to dive too much into this, but I feel your thinking is a little misplaced including what the BEAM does as “defensive.” It’s not a “wrong” way to think about it I suppose but then it would be more “The BEAM is defensive so we don’t have to be.”

To expand on @dimitarvp without the static typing angel, the idea is that we don’t want to handle errors that we can’t do anything about. Context matter greatly here! So taking your example of parsing options, it depends on—among probably other considerations—if you it’s library code or not. If it’s library code then whether or not you want to break it out into its own function, you should be caseing it and giving the user a better error message than the default “No match” error along the lines of what Dimi is suggesting. Really I agree wholeheartedly with him that we should all just be using NimbleOptions Otherwise, if this is code you control used by other code that you control then this comes back to what others have said above: you’re handling this at the wrong level. You’re basically just making better dev errors for yourself which is exactly the type of code The Erlang Way™ discourages.

For example if I have this:

def get_user!(id, opts \\ []) do
  preload = Keyword.get(opts, :preload, [])

  User
  |> Repo.get!(id)
  |> Repo.preload(preload)
end

And this is my callsite in, let’s say, a LiveView:

def mount(%{"id" => id}, _session, socket) do
  user = get_user!(id, preload: "invalid preload value")

  # ...
end

The bug is in the LiveView, not get_user. Having get_user add “extra safety” is pretty useless here. Once you fix that bug, the checking code will never error again. Remember we’re talking business logic and not a library here—the value being being provided at the callsite constant, we control it. Worrying about future uses of the function is jumping firmly into YAGNI territory. So in the end, the only thing this code is accomplishing is giving better errors in development. To me this isn’t worth the cost. I know how to read stacktraces and I would much rather not have to “read around” defensive code everywhere. As touched on, this is stuff that dialyzer and tests should be responsible for catching.

Lastly, if the code being passed in is untrusted, that is when you want to parse, don’t validate. Ensure all untrusted data has been cast into safe, expected data before any business function touches it. This way you can be sure they’ll get the right types and keep their code business-focused.

Again, I’m just trying to answer your initial question. The link I posted explains it and there is further reading on a mailing list from Joe Armstrong. But ya, totally free to program how you want—if you’re on a team that wants to write code like that, that’s all good!

[D4no0]

I would love for us someday to formalize on how to approach these kind of situations, because as someone who wrote quite a bit in languages like Kotlin and Java, coming back to elixir after using those I always feel like I need to rearrange my brain .

I am not sure how to convey this properly however writing in some of the languages like Java is like driving a vehicle and in elixir is like playing on a guitar, completely different concepts even if there is common syntax in those programming languages.