I rarely use ‘then’ for anything other than reordering arguments for a pipeline. I find that I end up having the same code that is in the ‘then’ in multiple place and leads me to create a function, with an intention revealing name, instead of using ‘then.’
The new functions lead to more readable and quicker grokking of the code on future passes. I’m always trying to code for future me and future others to speed up understanding of the code. I don’t find that this increases understanding. I think it would lead to more use of it and decrease the amount of time it takes future developers to grok the code. On Aug 5, 2024, at 07:48, Caio Oliveira <caio...@gmail.com> wrote:
Good idea! I searched through github to find more usecases like that to corroborate this change, and here's what I found:
I also noticed that most of the uses didn't need the predicate to be a function (and I think in this cases maybe using `|> case do` is even better, and the missing `else` clause won't be a problem here), so maybe this could be even simpler by just taking a value as the condition instead.
Also I'm ok with the `then_if` implementation, but I personally still prefer the `then(..., if: pred)` as it reads better to me, but I understand it might be too different from the rest of the kernel code. Coming back to some of my projects, I indeed found a bunch of similar cases, e.g. maybe_put_assoc, so even if I'm still on the fence I do see your point.
Assuming we introduce it, I'd find it more natural API-wise to have the condition first and then the action, which might be why the nested `if` felt more natural to me (consistent with `if`, other conditionals, and also Clojure's cond). |> then_if(fn_ -> modified_since end, &Req.put_header(&1, "If-Modified-Since", modified_since))
Yup, that’s what I usually do, but I see this in many places. `maybe_put_header`, `maybe_prepend`, etc., which makes me think that an abstraction would be convenient. And yeah, I wrote a `maybe_then` initially, but thought it was simple and convenient enough to be in the kernel.
Thank you for providing a concrete example.
Also subjective but I find the following more readable
|> then(&if(modified_since, do: Req.put_header(&1, "If-Modified-Since", modified_since), else: &1))
than
|> then(&Req.put_header(&1, "If-Modified-Since", modified_since), if: fn_ -> modified_since end)
But perhaps this case might benefit from introducing a private function
rather than relying on `then`, especially if this is a recurrent
use-case in your module/project:
defp maybe_add_header(req, _key, nil), do: req
defp maybe_add_header(req, key, value), do: Req.put_header(req, key, value)
...|> maybe_add_header("If-Modified-Since", modified_since)|> maybe_add_header("X-Entity-Id", entity_id)
It
should also be easy define your own then_if/3 macro if you really prefer the clojure style.
True, although personally I find the one-line `if` kind of confusing, especially when I was newer to the language.
> Additionally, your suggestion only implies what happens if `pred` is falsy while mine is clear.
This almost convinced me, to be honest! But on the flip side this makes it possible for you to forget to add a `else` clause, and just end up with a `nil` that is potentially hard to find where it came from.
Jose replied this in the PR (including here to centralize the discussion and not spam the PR and his email there):
> I personally would prefer to write the original code or use no pipeline at all. I don’t think the gain in conciseness justifies the loss in readability.
I'd say I agree with it 99% of the time, but there's this 1% that makes me miss Clojure's `cond->`. The concrete example that made me write this PR was this: I'm writing a small internal library to build requests for a third party service. There are some options that, if included, requires me to add headers to a request. The code looks something like this:
```elixir def request_entity(opts) do modified_since = Keyword.get(opts, :modified_since) entity_id = Keyword.get(opts, :entity_id)
|> add_x() |> authorize() |> add_body() |> then(&if(modified_since, do: Req.put_header(&1, "If-Modified-Since", modified_since), else: &1)) |> then(&if(entity_id, do: Req.put_header(&1, "X-Entity-Id", entity_id), else: &1))
And I'd much rather write something like this instead:
```elixir def request_entity(opts) do modified_since = Keyword.get(opts, :modified_since) entity_id = Keyword.get(opts, :entity_id)
|> add_x() |> authorize() |> add_body() |> then(&Req.put_header(&1, "If-Modified-Since", modified_since), if: fn_ -> modified_since end) |> then(&Req.put_header(&1, "X-Entity-Id", entity_id), if: fn _ -> entity_id end)
You can see conciseness is not the point*, but readability, robustness and convenience (a very subjective feeling, so feel free to ignore the last one).
Lastly, I know I could change the code in a million ways to avoid the pattern altogether, maybe even resulting in a cleaner result, but I feel this small addition would be a nice to have, and is something I miss, even if rarely.
* I left the conciseness out of the picture because I think it's way less important, but it does play a bit of a part. The actual example ends up needing to break the `if` into more lines, which doesn't read as good in the middle of the piping. You can already capture the `if` and do it as a one-liner
x |> then(&if(pred(&1), do: f(&1), else: &1))
so you don't gain much yet add more complexity. Additionally, your suggestion only implies what happens if `pred` is falsy while mine is clear.
-Greg Vaughn
> On Aug 3, 2024, at 4:16 PM, Caio Oliveira <cai...@gmail.com> wrote:
>
> x
> |> then(fn val ->
> if pred(&1) do
> f(val)
> else
> val
> end)
>
> Into this:
>
> x |> then(&f/1, if: &pred/1)
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/uM_M-DWh42A/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CANnyohay83mzBJRgz%3Dy8RZ6cp257u4t8zSfyMKMOfqmSTMvY2A%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAJLj4H9%3DsUxyupijoKeJbY1TVYQbjoGF7ALzG9_UT8LF8d655A%40mail.gmail.com.
You received this message because you are subscribed to a topic in the Google Groups "elixir-lang-core" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/elixir-lang-core/uM_M-DWh42A/unsubscribe.
To unsubscribe from this group and all its topics, send an email to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CANnyohZzP5KK-JoXV4BdSg4oGjzgW4AfxOeDu6V7SDqBopR0Ww%40mail.gmail.com.
--
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/CAJLj4H8tdQsK_bpxJXaYQawdLLOsYFt%2BKLL28yDqqbv1rUNP5A%40mail.gmail.com.
You received this message because you are subscribed to the Google Groups "elixir-lang-core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-lang-core+unsubscr...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-lang-core/28FA3E4F-762F-40C9-864B-053CC20F4938%40binarynoggin.com.
|
