Skip to content

fold_first API Change Proposal #157

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
conradludgate opened this issue Jan 1, 2023 · 4 comments
Closed

fold_first API Change Proposal #157

conradludgate opened this issue Jan 1, 2023 · 4 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api

Comments

@conradludgate
Copy link

conradludgate commented Jan 1, 2023

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

rust-lang/rust#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 conradludgate added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jan 1, 2023
@conradludgate conradludgate changed the title (My API Change Proposal) fold_first API Change Proposal Jan 1, 2023
@scottmcm
Copy link
Member

scottmcm commented Jan 1, 2023

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.

@the8472
Copy link
Member

the8472 commented Jan 2, 2023

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.

@Nemo157
Copy link
Member

Nemo157 commented Jun 15, 2023

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)

@the8472
Copy link
Member

the8472 commented Jul 27, 2024

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 the8472 closed this as not planned Won't fix, can't repro, duplicate, stale Jul 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api
Projects
None yet
Development

No branches or pull requests

4 participants