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!!