This is a beauty. Seen in the wild, not from me or my colleagues.
interval
|> Kernel.-(time_elapsed)
|> max(0)
|> schedule_events()
17 Likes
It is much harder to grasp than
remaining_time = max(0, interval - elapsed_time)
schedule_events(remaining_time)
10 Likes
Given many developers proclivity towards inline functions (of any size) I suspect that in a “pipeless” environment
schedule_events(max(interval - time_elapsed, 0))
would have been the likely result (which I find less clear (harder to parse) than the two line version).
I agree that Alco’s forumulation is best, but for short phrases its idiomatic to avoid unnecessary assignment, and pipes are the best way to do that in many cases.
More readable than the nested calls is:
max(0, interval - elapsed_time) |> schedule_events()
Which, seems fine to me, but Credo will give me shit about not starting a pipe with a value so I have to write
0 |> max(interval - elapsed_time) |> schedule_events()
I don’t think that’s much better than what we started with.
3 Likes
Unnecessary from what perspective?
remaining_time = max(0, interval - elapsed_time)
Sure the computer doesn’t need it but:
p. 15, Refactoring: Improving the design of existing code; 1999
The right hand side of the assignment focuses on what needs to be done - the left hand side enlightens us why it’s being done.
Sometimes I wonder whether these “idioms” date back to when this was normal
z = max(0, x - y)
schedule_events(z)
That z is unnecessary.
Pipes can be similarly affected by bad or lack of naming which is what is really going on in the OP.
interval
|> calc_remaining_time(time_elapsed)
|> make_positive_value()
|> schedule_events()
which should really become
interval
|> remaining_time(time_elapsed)
|> schedule_events()
but even that seems forced compared to
remaining_time = max(0, interval - elapsed_time)
schedule_events(remaining_time)
3 Likes
Unnecessary from the programmer’s perspective, of course. In some cases it’s necessary to make code understandable, but it isn’t always of course - that’s why the operator exists.
1 Like
I wouldn’t say the operator is to avoid assignment. The operator is to simplify nested calls. 
4 Likes
Isn’t nesting a means to avoid assignment? At least, assignment is the other alternative to nesting.
1 Like
One issue (problem) with the pipe is that in one way it hides what you are doing. Yes, you can see the actual operations being done but it can also hide what the actual data along the way is. Yes, you can add comments. However, using assignments means that you automatically do get information about what the actual data along the way is (at least if you use reasonable variable names).
I am not saying that you should avoid pipes and always use assignments, but I do think you can go too far with long pipe sequences.
This gets back to what I think is a very important question: for whom and why are you writing this code? Is it a quick hack which you don’t expect to have a long life? If it is for a product you envisage to be in use a long time which other people will maintain and develop then it is very important that you write clear, easily understandable and very explicit code. In this case maybe using judiciously using assignments can be a Good Thing™. I try to ask myself “in six months time will I understand what i have done here?” [*]
What I was really poking fun at was how far they had gone with the Kernel.- to be available to use a pipe.
[*] This also explains my opinion on having too many implicit default values which I can vent in a later post.
10 Likes
Another point to consider: pipelines are quite clear when a new version of the same thing is being passed along to each function, but when the context of what is being passed changes midway through, that makes it hard to read. A Plug.Conn being passed through adding headers, setting status, body, sending, etc is the perfect example of a good use.
In your example, even though it’s a number being passed, what that number represents is different at each stage. And the fact that foo |> Kernel.-(bar) is vastly less clear than foo - bar means you’re starting off the pipeline with the very first step obscuring rather than clarifying.
8 Likes
That’s a good point, just recently in a test I moved the ending of the pipe into a separate call cause it made it unclear as to what the input is, so the whole test looked confusing despite the nice looking pipeline 
I still think the pipes are awesome, but they definitely can be abused.
You can go too far even with short pipe sequences! For example, I have seen some code that does this:
def foo(list) when list |> is_list() do
which I am not a big fan of! 
Just to clarify, because it is slightly ambiguous, Kernel.- was not made available to be used as a pipe. All of our auto-imported functions are defined in Kernel. In this way it is similar to erlang:'-'(1), except Elixir considers all operators to also be valid identifiers (i.e. they don’t require quotes around them).
5 Likes
Oh fully realise this! Sometimes you need to be able to access all the built-ins as “normal” functions and not as operators, but perhaps not here. 
You yourself already adressed this two sentences later but in my eyes I should do my best to write production-grade code even for my hobby projects. I can stick to ugly and hardly-understandable code until I make something hard [for me] to eventually work but my next session immediately starts with refactoring – which also improves the understanding of any code you refactor anyway.
Indeed, asking yourself if you will understand what you were writing N weeks or months down the line is the right question.
Well to be fair this can be said about any language and any framework/library and feature they have. Don’t even ask me which crazinesses I’ve witnessed in my PHP and Java days – I’ll legitimately be a happier person if somebody wipes them off my brain, it can replenish some long-lost naive optimism that I sometimes desperately need.
1 Like
Oh yes, there are things in most languages which people misuse. In Elixir many are in love with pipes and some overuse them. Another non-Elixir specific example of things which are often misused are macros. Don’t get me wrong, as an old lisper I love macros, but they are both the road to heaven and the road to hell.
4 Likes
An interesting discussion. I also don’t like Kernel.-(time_elapsed) in a pipe.
I think it is difficult to say which length on a pipe is good an which is bad. Because it depends on the purpose of a pipe and the transformation of the data.
If I have to review the code snippet we had start with I would suggest something like this.
schedule_events(interval - time_elapsed)
@spec schedule_events(int) :: {:ok, ...} | {:error, ...}
def schedule_events(time) when time >= 0 do
...
end
def schedule_events(past), do: {:error, :missing_timemachine}
No pipe and the function takes control of its argument.
1 Like
Absolutely.
It probably would be a good candidate for with construct rather than pipe operator for readability and possibility of adding constraints/error handling/crashing when we want to.
Nowadays I find myself using with more and more and I often start with a pipeline just to refactor it afterwards to more explicit with syntax.
How would your with approach look like?
Wow: it’s really interesting to see this discussion. I wrote a whole post on/around this: https://www.erlang-solutions.com/blog/to-pipe-or-not-to-pipe.html
3 Likes
