Conditional piping part of the language?

[dimitarvp]

To elaborate with an example of my own, this I consider noisy code that over-compensates for something we can avoid:

defmodule Accept.List do
  def work(nil), do: {:error, :invalid}
  def work([]), do: {:ok, :nothing_to_do}
  def work([first | rest]), do: {:ok, :we_have_done_stuff}
end

This introduces two problems:

1. We have to think of a return value (or a logging event, APM notification etc.) for when stuff is nil when it should not be nil. Mind you, this might be desirable for some projects and I’ve been part of them. When this is something you are interested in then yes the above code is good. Has been a rarity in my practice but I recognize that it does happen.
2. We pollute this (and likely other) module with nil clauses for functions when it’s very likely we can protect against nil (or convert it to a default empty value e.g. [] for lists) at the call-site e.g. list_or_nil |> List.wrap() |> Accept.List.work().

Whereas, if we remove the nil clause above we’ll make our caller blow up with an error, thus making it very clear that we’ve made a mistake (vs. logging stuff or putting a non-fatal message in an APM system might not draw the programmer’s attention). Whether that mistake can be worked around with a default value or that’s truly an error depends on the project but I’d still prefer to “hear” about an invalid input to my function(s).

---

Of course this is all bike-shedding, more or less. I’m still defaulting to things blowing up vs. doing too much defensive programming and ending up with code that has no idea what to do with the hot potato when it lands on its lap.

[ityonemo]

we’ve deviated considerably from the main point, but I don’t think it’s bikeshedding and helping people craft clear code is important enough to merit discussion.

In the specific example you gave, I think if you’re creating a work function in a module called Accept.List, it probably shouldn’t take “nil” as a parameter defensively. It should only take lists. Already from how it’s been coded I can tell that the upstream caller is using get inappropriately and should instead be using, fetch! or fetch function instead. Understandably, not everyone follows these naming conventions, naming is hard and even I screw this up… a lot… in my private code, and periodically i have to go thorough and clean them up. Fetch is too tempting a name to refer to “going some long distance and getting data” instead of “this returns error/ok tuples”, but I do also think that respecting conventions is important.

if you have a function that can emit nil, you should probably be immediately consuming it in something that can deal with nils, and usually something that comes from stdlib/stdlib adjacent, so if, cond, to_string, List.wrap. Phoenix.HTMLSafe, etc.

For example, this is a pattern I use a lot:

Enum.flat_map(list_of_selected_keys, &List.wrap(if value = Map.get(map, &1), do: value))

That one could be a matter of taste because you can certainly do a filter on the list of selected keys, noting that the filter alternative does result in you potentially iterating through the list of keys twice.

But the one that is hard is when you have a cond:

cond do
  attempt1 = get_from_source_1(key) -> attempt1
  attempt2 = get_from_source_2(key) -> attempt2
  attempt3 = get_from_source_3(key) -> attempt3
  ...
end

I somewhat dislike doing a reverse with, which is the most legible alternative:

  with :error <- fetch_from_source_1(key),
         :error <- fetch_from_source_2(key)
  ...
  else
   {:ok, result} -> result
  end

I think it is a good thing to be used to reading railroad code as a list of successes. I find that reading over code that I myself wrote where I have a reverse with, even though I always put a comment warning me that this with statement is a reverse with, will still cause my brain to trip up and have to recontextualize, so I have stopped writing reverse withs.

There’s another List.wrap example

developing_list = [...] ++
  List.wrap(if must_be_2nd_last = Keyword.get(opts, :foo), do: must_be_2nd_last) ++
  List.wrap(if must_be_last = Keyword.get(opts, :bar), do: must_be_last)

if you only had one, you could do it pretty easily with a case statement and Keyword.fetch, avoiding nils, but it gets hairy with more than one.

[dimitarvp]

@ityonemo

Enum.flat_map(list_of_selected_keys, &List.wrap(if value = Map.get(map, &1), do: value))

Why not this?

Enum.flat_map(list_of_selected_keys, &Map.get(map, &1, []))

[aziz]

@derek-zhou It’s interesting you can do that but very unintuitive if you ask me.
Btw, you can probably simplify the quote expression like this:
unquote(name) = unquote(value) || unquote(name)

[aziz]

@ityonemo

There’s another List.wrap example

I don’t wanna get involved in the sidetracked discussion, but I was curious what your example would look like with the then?/3 macro:

[]
|> then?(must_be_2nd_last = opts[:foo], &[must_be_2nd_last | &1])
|> then?(must_be_last = opts[:bar], &[must_be_last | &1])

vs.

developing_list = [...] ++
  List.wrap(if must_be_2nd_last = Keyword.get(opts, :foo), do: must_be_2nd_last) ++
  List.wrap(if must_be_last = Keyword.get(opts, :bar), do: must_be_last)

The then? version looks way more readable and cleaner to me. What do you think?

[ityonemo]

I’m looking at the code that I have and actually it’s this, no need for if, silly me:

Enum.flat_map(..., &List.wrap(Map.get(map, &1))

You need the List wrap, because flat_map needs to have the lambda return lists.

For the ++ List.wrap... example, if you use cons operator, the order will be wrong. This is something I use when, e.g. building AST because the order of those lists matters a lot.

[derek-zhou]

Not the same (list order). I prefer:

developing_list = List.flatten([
[...],
List.wrap(Keyword.get(opts, :foo)),
List.wrap(Keyword.get(opts, :bar))
])

[aziz]

@ityonemo

For the ++ List.wrap... example, if you use cons operator, the order will be wrong. This is something I use when, e.g. building AST because the order of those lists matters a lot.

I see. In that case |> Enum.reverse() at the end would be enough, right?

@derek-zhou

Not the same (list order). I prefer:
…

Also a possible solution.

[sodapopcan]

Wanting a dedicated conditional pipe construct still feels like a smell to me. I could be wrong, of course! Does anyone have any concrete examples (ie, that aren’t speaking abstractly and using identifiers like foo and condition)?

Stuff I come across:

I often have a :preload option in my context functions that make db calls:

def list_posts(opts \\ []) do
  preload = Keyword.get(opts, :preload, [])

  Post
  |> Repo.all()
  |> Repo.preload(preload)
end

Passing an empty list to Repo.preload is a a no-op so the conditional just lives in Keyword.get/3.

Another thing would be filters in which case I use a reduce which gives me a reusable, pipeable filter function:

def apply_filters(query, filters) do
  Enum.reduce(filters, query, fn filter, query_acc ->
    case filter do
      {:limit, limit} ->
        from q in query_acc, limit: ^limit

      {:order_by, order_by} ->
        from q in query_acc, order_by: ^order_by

      # etc...
    end
  end)
end

Better yet I just use Flop. Maybe possibly likely I should be using Ash but I still haven’t made the leap.

Recently I came across the situation where I have forms with nested children and they require at least one child, so only when there are no children yet, I want to show an empty field to the user. Again, I make a generic, reusable function:

def maybe_build_assoc(changeset, key, assoc_struct) do
  if get_assoc(changeset, key) == [] do
    put_assoc(changeset, key, [assoc_struct])
  else
    changeset
  end
end

The last example seems to be the sticking point. I do agree that the if thing, do: transform(thing), else: thing is a little clunky, but it lives tucked away in a helper file that I barely look at and when I do, it’s very nice and explicit.

My examples are, of course, heavily Ecto-y and I’m just wondering where a conditional pipe would shine as I’ve personally never even thought about needing one.

Otherwise I do like @aziz’s then? example expect that I would call it maybe_then. While it does read surprisingly nicely with the ?, ? functions in Elixir conventionally return booleans.

[dimitarvp]

I never bought into the whole conventions thing e.g. ? and !. What matters most is what your team finds convenient. In most Elixir project I was in there was no mindset of “our project can raise” or “our project must NEVER raise” so we kind of knew the default by heart and this was not enforced with syntax conventions.

RE: naming, I’d prefer maybe_then as well, it conveys 100% of what it [could] do right in the name because you should know what Kernel.then does and then prepending maybe_ to it makes it clear what you can expect.

A not-super-on-topic example is this Rust code that I wrote a few years ago:

pub fn time_from_millis(timestamp: i64) -> Option<DateTime<Utc>> {
    if timestamp == 0i64 {
        None
    } else {
        Some(Utc.timestamp(timestamp / 1_000, ((timestamp % 1_000) * 1_000_000) as u32))
    }
}

// ...
order.received_time.and_then(time_from_millis)

…which, if you know Rust, basically says “fetch this field that could be None (non-existing) but if it does exist, invoke this function on the value and return it; if not, return None” i.e. fallible chains of actions where short-circuiting does not require boilerplate. You can have a chain of 20 transformers but if something at step 3 returns an Err or None value, it will just fall through and you don’t have to do anything else except just having your .and_then and other fallible functions strung together.

[dorgan]

I find the idea of “this function can raise” rather silly, because 90% of your code can raise and I bet most people don’t see pattern matching in function heads as a source of FunctionClauseExceptions and don’t add a ! to them. I also don’t like to “color” my functions that way, it gets super messy when it taints all the callers and it’s not clear what you gain with that. I’d rather reserve the ! for the cases where you explicitly raise by providing a custom error message, or when you also provide an alternative non-bang function

On topic: I’ve seen helpers like tap_if_ok, do_if, etc, and I have no issue with that at all. Since it’s something conventional, I like that being implemented in userland rather in elixir itself, neither elixir nor erlang impose a particular structure to the atom or the number of tuples in the result tuple to make it viable to add such constructs to the language itself, in my opinion.

Maybe something that would make more sense to me would be this:

defmodule Utils do
  defmacro match(data, pattern, do: block) do
    quote location: :keep do
      case unquote(data) do
        unquote(pattern) -> unquote(block)
        other -> other
      end
    end
  end
end

import Utils

42
|> do_something()
|> match {:ok, val} do
  val
end

[dimitarvp]

@dorgan

I’d rather reserve the ! for the cases where you explicitly raise by providing a custom error message, or when you also provide an alternative non-bang function

Sure, but I had exactly zero situations in my Elixir career – 7 years now – where in app code I had to write separate non-raising and raising function variants. I view this idea as a strictly library practice.

@dorgan

I like that being implemented in userland rather in elixir itself, neither elixir nor erlang impose a particular structure to the atom or the number of tuples in the result tuple to make it viable to add such constructs to the language itself, in my opinion.

Absolutely, I agree. Some people are not happy that this is not obvious but work with the language and the ecosystem for a bit of time and it starts to become natural…

[dorgan]

@dimitarvp

Sure, but I had exactly zero situations in my Elixir career – 7 years now – where in app code I had to write separate non-raising and raising function variants. I view this idea as a strictly library practice.

Agreed on that, honestly I’d rather explicitly match on {:ok, value} and have that raise if it returns anything else, than write a ! version too. Using a ! function may be helpful in pipes but “escaping the monad” can be either dangerous, or lead to code that’s hard to refactor when you actually need to handle the error case but all the code depending on that line of code assumes it always succeeds…

I like how Rust makes that smelly by forcing you to .unwrap it

[stevensonmt]

I might be misunderstanding the OP, but it seems like a case for with statements to me.
I’ll steal an example from this blog post:

with true <- is_email_address?(email),
     true <- String.length(code) === 6,
     %EmailConfirmation{} <- EmailConfirmations.get_email_confirmation(email),
     nil <- EmailAddresses.get_email_address(email),
     {:ok, user} <- Users.create_user(data),
     {:ok, email_address} <- EmailAddresses.create_email_address(user, email) do
  ...
end

The example glosses over the else statement for when the condition is not met, but I think it still serves.

---

I’m leaving it but I just realized how terrible the wording is in the statement “a case for with statements”; very confusing in this context. Sorry.

[dimitarvp]

@dorgan

Agreed on that, honestly I’d rather explicitly match on {:ok, value} and have that raise if it returns anything else, than write a ! version too.

That’s how it should be, we the programmers love to discuss these things to death but in the end you arrive at your app’s user-facing code and you have to make sure you have covered for everything AND make sure to send logging and/or APM events to your appropriate systems if something is not (quite) right.

So I usually write most of the internal code in quite a lax manner and then do more exhaustive pattern matching in my highest level app code because there you can’t allow any slack. Users don’t want to see “app crashed” page, they want to see “this did not work, please try again shortly” and you want to receive a notification in the meantime.

@dorgan

I like how Rust makes that smelly by forcing you to .unwrap it

Yes, and this is HUGE. Rust forces you to mark this explicitly which helps a ton with static analysis tools AND allows you to grep for it and tighten the bolts down the road. A lot of people that work predominantly with dynamic languages can’t quite appreciate how important such aspects are before they dive deeper in the static strongly typed / native-code-compiled languages. I miss this every time I write Elixir.

[sodapopcan]

@dimitarvp

What matters most is what your team finds convenient.

Absolutely, I was just talking in terms of something like that making it into the language. I have also never written a custom ! function unless it’s calling a library ! function (like, Blog.get_post!). I personally always prefer ? return a boolean but I’m certainly not going to try and police teams I’m not on

I only quickly read the docs for Rust’s and_then but isn’t that a situation where you would want with in Elixir? I think what this thread is talking about is conditionally adding to a data structure or returning it untouched. Though I may have lost the plot a bit

[dimitarvp]

@sodapopcan

I only quickly read the docs for Rust’s and_then but isn’t that a situation where you would want with in Elixir?

Elixir’s with – and this is also aimed at @stevensonmt’s most recent comment from a few minutes ago – suffers from lack of expressivity. Here’s how I’d rewrite his with block:

with {:check_email, true} <- is_email_address?(email),
     {:check_code_length, true} <- String.length(code) === 6,
     {:get_email_confirmation, %EmailConfirmation{}} <- EmailConfirmations.get_email_confirmation(email),
     {:email_confirmation_should_not_exist, nil} <- EmailAddresses.get_email_address(email),
     {:create_user, user} <- Users.create_user(data),
     {:create_email_address, email_address} <- EmailAddresses.create_email_address(user, email) do
  ...
end

…because in the else block you can have a better idea what’s wrong i.e. you can pattern-match on {:check_email, false} and know exactly which step failed.

The code above is looking very much like an Ecto.Multi chain and ironically enough, I started using Ecto.Multi instead of with in no small amount of cases just for this reason. But IMO it does give you a better code readability.

To answer your question, code comprised of chains of and_then is more expressive and more readable than Elixir’s classic with usages.

[sodapopcan]

This gets back to context mattering here. In your with example that is definitely better suited to a Multi! I use with generally when each stage has the same return value, mostly :ok/:error tuples, then you don’t have to care exactly which one failed. I like with when doing side effects where the database isn’t involved.

I do get that with is sort of a hacky do notion or the like, and isn’t the prettiest (I wish the formatter would let me put the do on its own line but I’m over it), but I prefer when I can see a with and know “this is a procedure where something can go wrong” and see a pipeline and think “this is just a series of immutable data transformations”.

I just have PTSD of working with someone who insisted everything be a pipeline.

[stevensonmt]

Not familiar enough with Ecto but will absolutely be looking at that in the future.

With respect to with statements though, I would argue that your rewrite of the example is exactly what makes them so great. You can make them as expressive as you like. I particularly like that they allow you to handle the happy path in one chain but you can handle multiple unhappy paths in the same place. It’s like instead of having Option::Some(res) | Option::None or Result::Ok(res) | Result::Err(err) you have a de facto return type of

Happy(ness) 
| NotAnEmail(res) 
| WrongCodeLen(res) 
| NotConfirmed(res) 
| FailedCreateUser(res)
| FailedCreateEmail(res)

[dimitarvp]

@sodapopcan

I use with generally when each stage has the same return value, mostly :ok/:error tuples, then you don’t have to care exactly which one failed.

While this is factually correct, I’ve seen such real-world scenarios extremely rarely. Most of the time with is used when you want a pipeline of transformations and/or side effects, yes, but very often you do want to know what failed. After all, if your 7-step with pipe keeps failing at step 3 wouldn’t you the app dev want to know why?

@sodapopcan

I just have PTSD of working with someone who insisted everything be a pipeline.

Zealotry and pyramid hierarchy are a sad fact of life of our profession. I’ve successfully challenged and changed the minds of CTOs and VPs of Engineering but it was a rarity, most people default to ego or traumatic past experiences and will ride that until the heat death of the Universe to justify what they do.