Hi Amos! Yeah, creating functions with the clear intent is good and preferable. What I don't like is having to create these `maybe_` variants that don't really add any clarity to the code and just serve to wrap a conditional statement and make it "pipeable". Because of that it doesn't feel like the current API is serving to guide devs to writing better code, but just being more verbose.
Also I second your point of rarely using `then`, but as I said before, in that 1% case I need it I think it could be a tad more convenient. Em seg., 5 de ago. de 2024 às 10:11, Amos King <a...@binarynoggin.com> escreveu: > 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. > > Amos King > Binary Noggin > > 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: > > - 972 results for just `|> maybe_put`: > https://github.com/search?q=language%3AElixir+%22%7C%3E+maybe_put%22&type=code > - 214 results for `then(&if...`, where the majority's `else` clause was > just `&1`: > https://github.com/search?q=language%3AElixir+%22%7C%3E+then%28%22+%26if&type=code > > This *feels* statistically relevant, considering the occurrence of `|> > then` > <https://github.com/search?q=language%3AElixir+%22%7C%3E+then%22&type=code> > is ~6.2k. > > 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. > > Em dom., 4 de ago. de 2024 às 00:15, Jean Klingler <sabiw...@gmail.com> > escreveu: > >> 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)) >> >> >> Le dim. 4 août 2024 à 11:22, Caio Oliveira <caio...@gmail.com> a écrit : >> >>> 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. >>> >>> On Sat, 3 Aug 2024 at 22:42 Jean Klingler <sabiw...@gmail.com> wrote: >>> >>>> 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. >>>> >>>> >>>> Le dim. 4 août 2024 à 09:59, Caio Oliveira <caio...@gmail.com> a >>>> écrit : >>>> >>>>> 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->` >>>>> <https://clojuredocs.org/clojure.core/cond-%3E>. 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) >>>>> >>>>> Req.new(url: "example.com") >>>>> |> 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)) >>>>> |> Req.request() >>>>> end >>>>> ``` >>>>> >>>>> 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) >>>>> >>>>> Req.new(url: "example.com") >>>>> |> 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) >>>>> |> Req.request() >>>>> 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. >>>>> Em sábado, 3 de agosto de 2024 às 19:09:10 UTC-3, gva...@gmail.com >>>>> escreveu: >>>>> >>>>>> 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 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/3c468ed7-1db6-46e3-bf23-45c21e501b3bn%40googlegroups.com >>>>> <https://groups.google.com/d/msgid/elixir-lang-core/3c468ed7-1db6-46e3-bf23-45c21e501b3bn%40googlegroups.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >>>> 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 >>>> <https://groups.google.com/d/msgid/elixir-lang-core/CANnyohay83mzBJRgz%3Dy8RZ6cp257u4t8zSfyMKMOfqmSTMvY2A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> -- >>> 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 >>> <https://groups.google.com/d/msgid/elixir-lang-core/CAJLj4H9%3DsUxyupijoKeJbY1TVYQbjoGF7ALzG9_UT8LF8d655A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>> . >>> >> -- >> 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 >> <https://groups.google.com/d/msgid/elixir-lang-core/CANnyohZzP5KK-JoXV4BdSg4oGjzgW4AfxOeDu6V7SDqBopR0Ww%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> > -- > 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 > <https://groups.google.com/d/msgid/elixir-lang-core/CAJLj4H8tdQsK_bpxJXaYQawdLLOsYFt%2BKLL28yDqqbv1rUNP5A%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > > -- > 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/28FA3E4F-762F-40C9-864B-053CC20F4938%40binarynoggin.com > <https://groups.google.com/d/msgid/elixir-lang-core/28FA3E4F-762F-40C9-864B-053CC20F4938%40binarynoggin.com?utm_medium=email&utm_source=footer> > . > -- 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/CAJLj4H9KN3aN__3YnuD21F%2B9wmzXGp0uE4_9CUUw_zvez2poNA%40mail.gmail.com.
