Discussion #1: Should we add map_get/2 and is_map_key/2 to Kernel?
Yes, we should. There are things that can’t be expressed with the other proposals (or any other way in a guard for that matter), most notably:
def check_key(map, key) when is_list(map_get(map, key)), do: ...
It’s effectively implementing the def foo(key, %{key => value}) ... that is such a frequent request - now we’d have a way to do this.
However, I’m not a huge fan of the map_get name (pretty ironic considering it’s me who contributed it to Erlang). It feels very “imperative” while I find pattern matching and guards to be mostly about declarative code.
For example, for tuples, we use elem, not get_elem and for lists hd not get_hd (though head would probably be better). Because of that, I think we should explore adding this function under some different name. Some proposals: field, map_field, map_value, key_value, …
Discussion #2: Should we allow patterns in defguard/1?
Yes, I think we should. I like this idea a lot.
The only problem is that we might hit some performance issues with complex patterns when the value is used multiple times. Since we can’t bind inside guards, we’d have to generate the “access” code at each place the value is used. It becomes especially problematic when coupled with the in operator. It might lead to gigantic code explosion. I don’t think compiler would be smart enough to eliminate all those common sub-expressions (a long-term solution to this would be compiling to core erlang that allows binding in guards, that’s another set of issues, though).
With all of that said, I don’t think this should prevent us from exploring the implementation of this feature.
Discussion #3: Should we allow map.field in guards?
No. This feature would break the consistency of the language and make some construct mean different things inside and outside guards.
It’s true, today there’s difference in how code fails inside and outside guards (e.g. hd([]) might fail with an exception or just fail the current guard clause). When the expression fails or succeeds is always the same, though - this would break it and break it completely silently.
As a counter example, I could easily imagine somebody trying to write code like that:
def update_or_rollback(repo, changeset) do
case repo.update(changeset) do
{:error, reason} when repo.in_transaction?() -> repo.rollback(reason)
other -> other
end
end
Which would just silently fail. And it’s not a completely artificial example - in one of the projects I have a very similar function:
def update_or_rollback(repo, changeset) do
case repo.update(changeset) do
{:error, reason} = error ->
if repo.in_transaction?(), do: repo.rollback(reason), else: error
other -> other
end
end
