-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add Option::inspect
and Result::{inspect, inspect_err}
#91346
Conversation
ibraheemdev
commented
Nov 29, 2021
•
edited
Loading
edited
r? @dtolnay (rust-highfive has picked a reviewer for you, use r? to override) |
Would you be willing to consider using the |
I think |
If you're using functional methods on the Option/Result like this, it's likely part of a method call chain (like a pipeline). In that case I think tap is appropriate, since you are not just inspecting the value, you are keeping it going through the pipeline as you do so. Just tapping it, not taking it out to inspect. |
Of course, I didn't mean to say otherwise. :) |
fwiw I'd prefer they be named |
I'm a little surprised the signature isn't fn inspect<F: FnOnce(&T)>(&self, f: F); though I understand the benefit of method chaining. How useful would method chaining actually be in practice? Sometimes you have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Thanks for including reasonably representative example code which conveys the character of where you might find these functions used in real code, i.e. as opposed to something minimal-effort / contrived like let result = Ok(1); result.inspect(|x| foo(x));
.
@bors r+ |
📌 Commit 2e8358e has been approved by |
…askrgr Rollup of 7 pull requests Successful merges: - rust-lang#87160 (When recovering from a `:` in a pattern, use adequate AST pattern) - rust-lang#90985 (Use `get_diagnostic_name` more) - rust-lang#91087 (Remove all migrate.nll.stderr files) - rust-lang#91207 (Add support for LLVM coverage mapping format versions 5 and 6) - rust-lang#91298 (Improve error message for `E0659` if the source is not available) - rust-lang#91346 (Add `Option::inspect` and `Result::{inspect, inspect_err}`) - rust-lang#91404 (Fix bad `NodeId` limit checking.) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…Sushi Stabilize `result_option_inspect` This PR stabilizes `result_option_inspect`: ```rust // core::option impl Option<T> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; } // core::result impl Result<T, E> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; pub fn inspect_err<F: FnOnce(&E)>(self, f: F) -> Self; } ``` <br> Tracking issue: rust-lang#91345. Implementation PR: rust-lang#91346. Closes rust-lang#91345.
Stabilize `result_option_inspect` This PR stabilizes `result_option_inspect`: ```rust // core::option impl Option<T> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; } // core::result impl Result<T, E> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; pub fn inspect_err<F: FnOnce(&E)>(self, f: F) -> Self; } ``` <br> Tracking issue: rust-lang/rust#91345. Implementation PR: rust-lang/rust#91346. Closes rust-lang/rust#91345.
Stabilize `result_option_inspect` This PR stabilizes `result_option_inspect`: ```rust // core::option impl Option<T> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; } // core::result impl Result<T, E> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; pub fn inspect_err<F: FnOnce(&E)>(self, f: F) -> Self; } ``` <br> Tracking issue: rust-lang/rust#91345. Implementation PR: rust-lang/rust#91346. Closes rust-lang/rust#91345.
Stabilize `result_option_inspect` This PR stabilizes `result_option_inspect`: ```rust // core::option impl Option<T> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; } // core::result impl Result<T, E> { pub fn inspect<F: FnOnce(&T)>(self, f: F) -> Self; pub fn inspect_err<F: FnOnce(&E)>(self, f: F) -> Self; } ``` <br> Tracking issue: rust-lang/rust#91345. Implementation PR: rust-lang/rust#91346. Closes rust-lang/rust#91345.