Skip to content

Document additional use case for Iterator::inspect #49564

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
nickbabcock opened this issue Apr 1, 2018 · 4 comments
Closed

Document additional use case for Iterator::inspect #49564

nickbabcock opened this issue Apr 1, 2018 · 4 comments
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-iterators Area: Iterators E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.

Comments

@nickbabcock
Copy link
Contributor

nickbabcock commented Apr 1, 2018

If you'd like to fix this bug, here's how.


Currently iterator::inspect contains the following documentation:

It's much more common for inspect() to be used as a debugging tool than to exist in your final code, but never say never.

Though I like the never say never wording, I think it might be helpful to document a use case where it's not used for debugging. I use inspect in daemon applications for that slurp in stdin lines and discard and log the lines that can't be parsed. Like so:

lines
.iter()
.map(|line| parse_line(line))
.inspect(|line| {
    // If we can't parse a line, yeah that sucks but it's bound to happen so discard
    // the line after it's logged for the attentive sysadmin
    if let Err(ref e) = *line {
        error!("Parsing error: {}", e);
    }
})
.flat_map(|x| x)
// etc

Maybe the documentation of inspect can be updated to something like:

It's more common for inspect() to be used as a debugging tool than to exist in your final code, but applications may find it useful in certain situations as well. Say an application is reading stdin lines, it may be it helpful to log lines with errors before discarding them.

What do you think? Suggestions certainly wanted 😄

@frewsxcv frewsxcv added the A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools label Apr 1, 2018
@scottmcm
Copy link
Member

scottmcm commented Apr 2, 2018

Well, another possibility (YMMV) would be to combine the inspect+flat_map into a filter_map:

lines
.iter()
.map(|line| parse_line(line))
.filter_map(|line| match line {
    Ok(x) => Some(x),
    Err(e) => { error!("Parsing error: {}", e); None },
})
// etc

FWIW it took me a second to remember that flat_map over a result does what it does, so I think I prefer the explicitness of the filter_map overall.

@Centril Centril added the A-iterators Area: Iterators label Apr 3, 2018
@nickbabcock
Copy link
Contributor Author

Yeah, you can use filter_map(Result::ok) as well

lines
.iter()
.map(|line| parse_line(line))
.inspect(|line| {
    if let Err(ref e) = *line {
        error!("Parsing error: {}", e);
    }
})
.filter_map(Result::ok)

Sorry, I wasn't trying to draw attention to flat_map 😅

@steveklabnik steveklabnik added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels May 28, 2018
@steveklabnik
Copy link
Member

I'm happy to mentor anyone who'd like to fix this bug! Here's how:

The documentation for this method lives here:

/// # Examples
///
/// Basic usage:
///
/// ```
/// let a = [1, 4, 2, 3];
///
/// // this iterator sequence is complex.
/// let sum = a.iter()
/// .cloned()
/// .filter(|x| x % 2 == 0)
/// .fold(0, |sum, i| sum + i);
///
/// println!("{}", sum);
///
/// // let's add some inspect() calls to investigate what's happening
/// let sum = a.iter()
/// .cloned()
/// .inspect(|x| println!("about to filter: {}", x))
/// .filter(|x| x % 2 == 0)
/// .inspect(|x| println!("made it through filter: {}", x))
/// .fold(0, |sum, i| sum + i);
///
/// println!("{}", sum);
/// ```
///
/// This will print:
///
/// ```text
/// 6
/// about to filter: 1
/// about to filter: 4
/// made it through filter: 4
/// about to filter: 2
/// made it through filter: 2
/// about to filter: 3
/// 6
/// ```

Add the final example by @nickbabcock above below all of the other ones.

I'm happy to answer any more questions!

@nickbabcock
Copy link
Contributor Author

I'll take it. I didn't know if there needed to be more discussion about the proposed use case.

If I don't figure things out, I'll come back here with question 😉

bors added a commit that referenced this issue May 29, 2018
Document additional use case for iter::inspect

Adds docs for `iter::inspect` showing the non-debug use case

Closes #49564
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Documentation for any part of the project, including the compiler, standard library, and tools A-iterators Area: Iterators E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.
Projects
None yet
Development

No branches or pull requests

5 participants