Suggestion of smell: piping into case statements

[DaniruKun]

Originally inspired by Good and Bad Elixir

Credit to @keathley for the code examples

While piping into case statements is valid code, in my opinion it hurts readability, because it makes it unclear what expression or value is exactly being matched on.

There are even examples where the result of a case statement is further piped into other functions, and makes it harder to reason about the data flow and leads to another smell: overly long pipe chains.

Take this example:

# Don't do...
build_post(attrs)
|> store_post()
|> case do
  {:ok, post} ->
    # ...

  {:error, _} ->
    # ...
end

After refactoring:

changeset = build_post(attrs)

case store_post(changeset) do
  {:ok, post} ->
    # ...

  {:error, _} ->
    # ...
end

It becomes clear that the result of store_post/1 is a changeset.

[fishtreesugar]

I would like to say: "It depends", if functions in pipeline w/ a meaningful name, or reader quite familiar with the context, piping into case is acceptable imo

attrs
|> Post.create_changeset()
|> Repo.insert()
|> case do
  {:ok, %Post{}} ->
    # ...

  {:error, %Ecto.Changeset{}} ->
    # ...
end

[zoedsoupe]

I would like to say: "It depends", if functions in pipeline w/ a meaningful name, or reader quite familiar with the context, piping into case is acceptable imo

Exactly! Although a case expression between pipes can be very confuse:

attrs
|> Repo.insert()
|> case do
  {:ok, _} -> # something A
  {:error, _} -> # something B
end
|> maybe_send_to_client()

I mean, in this case is better to refactor to a with expression

[bryannaegele]

This can be accomplished with then/2 and multiple function heads which removes piping to a macro.

build_post(attrs)
|> store_post()
|> then(fn
  {:ok, post} ->
    # ...
  {:error, _} ->
    # ...
end)

[josevalim]

Yeah, I don’t this is a smell either. I also think it mostly boils down to syntax preference and not really related to code design or code quality. :)

[lucasvegi]

Hi @DaniruKun,

I just replied to another issue quoting a snippet from @josevalim, which pretty much represents what I think. I will do it again now. 😃

...it mostly boils down to syntax preference and is not really related to code design or code quality

I understand that in general, "piping into case statements" does not harm the quality of the code or even decrease its performance, for this reason, it is not a code smell.

Reading everyone's comments, I confess that I'm more concerned about:

• overly long pipe chains
• (long) case expression between pipes (@zoedsoupe)

I think we can close this issue. If you want to detail these two possible smells that I mentioned above, feel free to open new specific issues ok?

Again, thanks for the contributions!