Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion of smell: complex else clauses in with #7

Closed
josevalim opened this issue Mar 28, 2022 · 5 comments
Closed

Suggestion of smell: complex else clauses in with #7

josevalim opened this issue Mar 28, 2022 · 5 comments
Labels
suggestion of smell Elixir-specific smells suggested by the community

Comments

@josevalim
Copy link
Contributor

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
@fishtreesugar
Copy link

fishtreesugar commented Mar 28, 2022

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

@josevalim
Copy link
Contributor Author

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

@tiagoefmoraes
Copy link

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

Just linking to the docs :)

beware

@lucasvegi lucasvegi added the suggestion of smell Elixir-specific smells suggested by the community label Apr 7, 2022
@lucasvegi
Copy link
Owner

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

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

@josevalim
Copy link
Contributor Author

Excellent!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion of smell Elixir-specific smells suggested by the community
Projects
None yet
Development

No branches or pull requests

4 participants