libs-team icon indicating copy to clipboard operation
libs-team copied to clipboard

fold_first API Change Proposal

Open conradludgate opened this issue 2 years ago • 3 comments

Proposal

Problem statement

Iterator::fold is an extremely useful API to process all the elements in an iterator. Sometimes you don't have a sensible initial value. This is where Iterator::reduce comes in. However, this removes some flexibility with the return type. It must be the same as the item stream.

There's a middle ground where you instead derive the initial fold value with the first element in the iterator.

Motivation, use-cases

The docs for fold presents this example

let numbers = [1, 2, 3, 4, 5];

let zero = "0".to_string();

let result = numbers.iter().fold(zero, |acc, &x| {
    format!("({acc} + {x})")
});

assert_eq!(result, "(((((0 + 1) + 2) + 3) + 4) + 5)");

There's no way to avoid the (0 + 1) situation with either fold or reduce unless you fold over a Option<String> or reduce over String iterators.

With fold_first, this is easy

let numbers = [1, 2, 3, 4, 5];

let result = numbers.iter().fold_first(
    |first| first.to_string(),
    |acc, &x| format!("({acc} + {x})"),
).unwrap();

assert_eq!(result, "((((1 + 2) + 3) + 4) + 5)");

Solution sketches

https://github.com/rust-lang/rust/pull/106348

Links and related work

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

conradludgate avatar Jan 01 '23 11:01 conradludgate

That example exists in the docs mostly to emphasize the order in which the accumulator is applied.

Can you put more about your real-life situations where this is something you need?

My instinct is that we might not want another of these consumers, especially with a called-at-most-once closure. reduce existing for the homogeneous case is one thing, but even that wasn't an obvious addition, showing up many years after the more critical fold. It might be fine for people to just do the next-then-fold approach for this themselves.

scottmcm avatar Jan 01 '23 20:01 scottmcm

The next-then-fold pattern feels like a more general tap-like thing where you have a pile of adapter calls, then want to apply something to that temporary and then want to apply something else.

the8472 avatar Jan 02 '23 14:01 the8472

I recently wrote some code that would have benefited from this API (and I'm pretty sure I've looked for it in the past too):

let mut messages = messages.into_iter();
let initial = Error::msg(messages.next().ok_or_else(|| eyre!("missing initial message"))?);
messages.fold(initial, |err, msg| err.wrap_err(msg))

with fold_first I find it much more readable to not switch back and forth between the imperative and functional APIs:

messages
  .into_iter()
  .fold_first(Error::msg, |err, msg| acc.wrap_err(msg))
  .ok_or_else(|| eyre!("missing initial message"))?

(code context: messages: Vec<String>, Error is eyre::Report, this is rebuilding a chained report from serialized messages sent over the network)

Nemo157 avatar Jun 15 '23 11:06 Nemo157

We discussed this briefly during this week's libs-api meeting. There weren't many strong opinions, but nobody wanted to second this addition, so we're going to close it.

The recommendation is to call it.next().map(|first| it.fold(first.to_string(), ...)) or let first = it.next(); it.fold(...).

the8472 avatar Jul 27 '24 00:07 the8472