CODEWITHSUNDEEP
elixir
Raphael Rafatpanah
Raphael Rafatpanah

Reputation: 19967

Comparison with nil returns true

It seems that the following expression evaluates to "true":

if nil["nonexistent"] > 0.7 do "true" else "false" end

What is the idiomatic way to prevent this type of bug? In my case, I have a map that may or may not be nil.

Upvotes: 0

Views: 316

Answers (2)

vaer-k
vaer-k

Reputation: 11743

There are many ways to do this, and I don't think any of them are particularly "the" idiomatic way to do it. In most cases I would simply ensure the type of the map, or the value of the key, using guards or pattern matching.

if is_float(mymap?["maybe_existent"]) do
  mymap?["maybe_existent"] > 0.7
else 
  false 
end

or

if is_map(mymap?) do 
  mymap?["nonexistent"] > 0.7
else 
  false 
end

or

case mymap? do
  %{"maybeexistent" => existent} when is_number(existent) -> 
    existent > 0.7
  _ ->
    false
end

or

with %{"existent" => existent} when is_number(existent) <- mymap? do
  existent > 0.7
else 
  _ -> false
end

or

def f(%{"existent" => existent), do: existent > 0.7
def f(_), do: false

I think in general this kind of problem occurs when several intermediary processing steps are mashed into one function. You'll often want to break up your logic to operate on your data in pipelines of smaller processes (I'm talking about "pipelines" in the abstract, not necessarily Elixir pipelines formed with the |> macro), and there's certainly nothing wrong with breaking out some logic into nice private helper functions.

So for example, you may have a list of data that may, or may not be, maps (maybe they could be nils, or something else entirely). You should split that question out into a separate part of the process pipeline, so that when it comes time to check on the value of your key, you already know exactly what values you're operating on:

things = [%{a: 1}, nil, %{a: 3}, nil, nil, %{a: 0}]

things
|> Stream.filter(&is_map/1)
|> Stream.filter(& is_number(&1.a))
|> Stream.filter(& &1.a > 0.7)
|> Enum.to_list()

# results in: [%{a: 1}, %{a: 3}]

This is a dynamically typed language, so you're free to use your discretion to decide on how careful you want to check for possible mismatched types or missing data. In the previous example, it's possible that the key :a doesn't exist in the map, and we didn't check for that, instead we chose to just assume it's there if we already know we're working on a map. Maybe you should provide stricter checks here. It's up to you. You could do that by using another filter, or maybe using well-defined structs instead of maps.

Or maybe you don't start with a collection of data at all, but instead you're just handed one data point (maybe from something external) and you need to check on the value of a key on it, then you might pass it through a series of defined functions that transform it:

def handle_in(:create_thing, %{"thing" => mymap?}, socket) do
  if thing_worthy?(mymap?) do
    thing = make_thing(mymap?)    
    {:reply, {:ok, %{thing: thing}}, socket}
  else
    {:reply, {:error, :create_thing_fail}, socket}
  end
end

defp thing_worthy?(mymap) do
  mymap
  |> my_guarantee_key("existent")
  |> my_exceeds_threshold?(0.7)
end

But otherwise I'd probably just use a case or an if statement with a couple guard checks, similar to one of the first examples above.

Upvotes: 4

Paweł Obrok
Paweł Obrok

Reputation: 23164

I'd like to offer an alternative solution: using Map.get instead of []. While [] can index a nil returning nil, Map.get (and other Map functions) will crash in those cases, allowing you to detect the bug:

iex(1)> Map.get(nil, "key")
** (BadMapError) expected a map, got: nil
    (elixir 1.10.3) lib/map.ex:450: Map.get(nil, "key", nil)
iex(1)> nil["key"]
nil

Then, if crashing is not what you want, you can use || to ensure the first argument is a map and provide some default:

iex(2)> x = nil
iex(3)> Map.get(x || %{}, "key", 0) > 7
false

Upvotes: 1

Related Questions