Prefer Option::map/etc over match wherever it improves clarity
#52658
Conversation
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors r+ rollup |
|
📌 Commit cbe5f1c has been approved by |
bors
added
S-waiting-on-bors
…s, r=cramertj
Prefer `Option::map`/etc over `match` wherever it improves clarity
This isn't intended to change behavior anywhere. A lot of times statements like `match x { None => None, Some(y) => [...] }` can be rewritten using `Option::map` or `Option::and_then` in a way that preserves or improves clarity, so that's what I've done here.
I think it's particularly valuable to keep things in `libcore` and `libstd` pretty/idiomatic since it's not uncommon to follow the `[src]` links when browsing the rust-lang.org docs for std/core. If there's any concern about pushing style-based changes though, I'll happily back out the non-std/core commits here.
| self.iter.next_back().map(|ch| { | ||
| let index = self.front_offset + self.iter.iter.len(); | ||
| (index, ch) | ||
| }) |
There was a problem hiding this comment.
The other possibility for code like this is to let ch = self.iter.next_back()?;, though I don't know if that's better, nor if it's canonical style.
There was a problem hiding this comment.
Ooh, I forgot ? had been extended to support more than just Result types. If you want, I can spin up another PR for that - as it looks like these commits were just merged - but otherwise I suppose I'll just leave this be.
There was a problem hiding this comment.
I'm not confident that it's better, so it's probably not worth bothering with changing -- going to something other than the manual match, as you did, is the important part here.
Rollup of 7 pull requests Successful merges: - #52391 (Add unaligned volatile intrinsics) - #52402 (impl PartialEq+Eq for BuildHasherDefault) - #52645 (Allow declaring existential types inside blocks) - #52656 (Stablize Redox Unix Sockets) - #52658 (Prefer `Option::map`/etc over `match` wherever it improves clarity) - #52668 (clarify pointer offset function safety concerns) - #52677 (Release notes: add some missing 1.28 libs stabilization) Failed merges: r? @ghost
This isn't intended to change behavior anywhere. A lot of times statements like
match x { None => None, Some(y) => [...] }can be rewritten usingOption::maporOption::and_thenin a way that preserves or improves clarity, so that's what I've done here.I think it's particularly valuable to keep things in
libcoreandlibstdpretty/idiomatic since it's not uncommon to follow the[src]links when browsing the rust-lang.org docs for std/core. If there's any concern about pushing style-based changes though, I'll happily back out the non-std/core commits here.