lucasvegi/Elixir-Code-Smells

Suggestion of smell: complex else clauses in with

josevalim opened this issue · 5 comments

It is a small to match on different else results in with:

with {:ok, encoded} <- File.read(...),
     {:ok, value} <- Base.decode64(encoded) do
  value
else
  {:error, _} -> :badfile
  :error -> :badencoding
end

That's because it is impossible to know from which clause the error value came from. Instead, you should normalize the return types in the clauses:

with {:ok, encoded} <- file_read(...),
     {:ok, value} <- base_decode64(encoded) do
  value
end

def file_read(file) do
  case File.read(file) do
    {:ok, contents} ->{:ok, contents}
    {:error, _} -> :badfile
  end
end

def base_decode64(contents) do
  case Base.decode64(contents) do
    {:ok, contents} ->{:ok, contents}
    :error -> :badencoding
  end
end

That's because it is impossible to know from which clause the error value came from. Instead, you should normalize the return types in the clauses:

there's a way to know error from which clause: 😂

with {:read_file, {:ok, encoded}} <- {:read_file, File.read(...)},
     {:decode, {:ok, value}} <- {:decode, Base.decode64(encoded)} do
  value
else
  {:read_file , {:error, _}} -> :badfile
  {:decode, :error} -> :badencoding
end

and highly recommend section "Avoid else in with blocks" in Good and Bad Elixir

@fishtreesugar yeah, That’s discouraged too. I think we even updated the docs to mention it. :)

That’s discouraged too. I think we even updated the docs to mention it. :)

Just linking to the docs :)

beware

I added this smell to the catalog as well. Looks good?

https://github.com/lucasvegi/Elixir-Code-Smells#complex-else-clauses-in-with

Excellent!!