What would you remove from Elixir?

Chat & Discussions Chat / Discussions
29

Turning to tuples everywhere, unfortunately, isn’t a viable solution. Just couple days ago, there was an issue about replacing the “awfully nil-ridden” Enum.find_value (and friends). This would mean, however, in many places replacing code that is very readable and simple with something very complex.

Let’s look at some examples I posted in the issue:

In Phoenix here we would change from:

content =
  Enum.find_value(sources, fn source ->
    File.exists?(source) && File.read!(source)
  end) || raise "could not find #{source_file_path} in any of the sources"

to

res = 
  Enum.seach(sources, fn source ->
    if File.exists?(source) do
      {:ok, File.read!(source)}
    else
      :error
    end
  end) 
content =
  case res do
    :error -> raise "could not find #{source_file_path} in any of the sources"
    {:ok, content} -> content
  end

Another similar use case is present just couple lines lower in the same file. I also found a couple more uses with an || after find_value/2 in Phoenix, but those could be arguably replaced by passing the default value to find_value/3. It’s not possible in case we wanted to raise, though.

Another use that would require more convolution would require changing from:

Enum.find_value(headers, fn({k, v}) -> k =~ ~r/^origin$/i && v end)

to

Enum.search(headers, fn({k, v}) -> if k =~ ~r/^origin$/i, do: {:ok, v}, else: :error end end)

with additional modification of the consumer, from a simple if to a case match on the ok tuple.

Last use I found after a brief grep of deps of my app is in ecto (and that one I wrote myself). It would require changing:

defp check_operations_valid(operations) do
  Enum.find_value(operations, &invalid_operation/1) || {:ok, operations}
end

defp invalid_operation({name, {:changeset, %{valid?: false} = changeset, _}}),
do: {:error, {name, changeset, %{}}}
defp invalid_operation(_operation),
do: nil

to
```elixir
defp check_operations_valid(operations) do
  case Enum.search(operations, &invalid_operation/1) do
    :error -> {:ok, operations}
    {:ok, invalid} -> {:error, invalid}
  end
end

defp invalid_operation({name, {:changeset, %{valid?: false} = changeset, _}}),
do: {:ok, {name, changeset, %{}}}
defp invalid_operation(_operation),
do: :error

While it does not introduce more code, it’s much harder to understand, since we’re looking for an error, but have to return it tagged in an ok tuple, just to switch tags at the end.

The situation is far more nuanced than “nil is bad”. I urge some of you that propose removing it, to try to remove it from your code and get back to us with results.

Also, switching from atom nil to [] would make the issue far, far worse. An empty list is so much more ubiquitous than atom nil, it would require that much more attention when used. It would also lead to conflating two notions - empty collections and “no value”.

4 Likes
With do(:)?