Proposal: More exclusions for "Pipe chain should start with a raw value"

[ryanwinchester]

What were you trying to do?

I want a way to disable the check if it is only a single pipe and on a single line, from one function to another.

For example, in another issue (#32) a user had this example:

result = Stream.chunk(1..100, 10) |> Enum.take(3)

That same pattern is useful in other places.

bar = List.wrap(foo) |> Enum.map(&my_func/1) 

empty? = Repo.one(query) |> is_nil()

etc...

These cases are still easily readable, and shouldn't need to be something like 4 lines when it can easily be one, if that's what you prefer.

Expected outcome

A way to disable suggested refactoring opportunities on a single pipe (on a single line) from one function to another.

Actual outcome

[F] → Pipe chain should start with a raw value.

[OvermindDL1]

These cases are still easily readable, and shouldn't need to be something like 4 lines when it can easily be one, if that's what you prefer.

A problem with it being on one line is that |> can visually vanish among all the other noise of the line.

[ryanwinchester]

If you have line-length checks then there shouldn't be that much going on on one line that a pipe wouldn't be noticeable.

[hickscorp]

I might have left my comment in the wrong issue: rrrene/credo#32 - I can remove it if it's preferable - but here it is again:

I would really like this to be extended to case statements too... For example:

    try do
      f.(data)
    rescue
      e -> {:error, {:ex, e, System.stacktrace()}}
    end
    |> case do
      {:ok, new_data} -> {:ok, {name, new_data}}
      err -> {err, state}
    end
    |> Tuple.insert_at(0, :reply)

looks really better than the non-pipe version. Am I missing something?

[rrrene]

looks really better than

@hickscorp This is very much your personal taste. I would agree that we can make any check configurable to be able to accomodate your personal taste, but there no such thing as "really better".

I would very much prefer this over your proposed solution:

result =
  try do
    f.(data)
  rescue
    e -> {:error, {:ex, e, System.stacktrace()}}
  end

case result do
  {:ok, new_data} -> 
    {:reply, :ok, {name, new_data}}
  err -> 
    {:reply, err, state}
end

since it separates the try-block and the interpretation of its result (and it avoids the confusing Tuple.insert_at/3 call).

But that is also just an opinion. 😉

[hickscorp]

@rrrene Gotcha.

At this point, anything discussed here just looks like a matter of personal taste anyways - you're suggesting to name things, then why wouldn't that also apply in the case of:

wrapped = List.wrap(foo)
mapped = Enum.map(wrapped, &my_func/1)

You get the point. IMO pipping into a case should be configurable (read allowable) somehow.

[ryanwinchester]

@hickscorp I agree, any sort of code style checking or formatting is going to boil down to tastes in the end.

However, there are decisions that can be made about what most people will find more easily readable and understandable. And yes, there will be some contention at the edges of these decisions (how often are the lines cut-and-dry clean?), but in general I think we can hash it out, and people will have to make compromises, but in the end most people can be mostly happy.

Piping, when used well really enhances the readability and displays the intent of the code much better than assigning temporary variables in multiple steps, or nesting function calls.

However, there is a point where piping can actually make it harder to scan/read/grok what is going on right away and I think your example is one of those cases. I agree with @rrrene here.

I would also like to note that your proposal is much different than mine and shouldn't be included here. My proposal is about one single pipe on one single line.

If you feel very strongly about your proposal, you should make your own issue and you can see if anybody agrees with you, or if anybody would make a pull request for it.