3 ms·
Iterator-chain version would be a lot better looking if it was actually was a chain and not everything pushed into a single `filter_map`. "Ifs" are weird lookin
by 0x457 4y ago
Iterator-chain version would be a lot better looking if it was actually was a chain and not everything pushed into a single `filter_map`. "Ifs" are weird looking, one branch is using `entry`(I take it's a `DirEntry`?) and another is using `file_name` I suspect both could use the and look like `Self::is_` ?
If you're going to write an iterator chain in an imperative way, then you might as well right it imperatively.
A few notes:
- you can compare `&OsStr` with `&str` without `to_string_lossy` any explicit conversion (`&str -> &OsStr` done in `PartialEq` is basically free)
- you can get extension from `Path` by using `Path::extension`
- you can get filename without extension by using `Path::file_stem` or `Path::file_prefix`
- making your parse functions take the same arguments makes it a lot cleaner
Something like this:
read_dir("")?
.map(|e| e.map(|e| e.path()))
.map(|path| {
path.and_then(|path| {
if Self::is_bundle(&path) {
self.parse_post_bundle(&path.file_name().to_string_lossy(), &path).map(Some)
} else if Self::is_markdown(&path) {
self.parse_post(&path.file_stem().to_string_lossy(), &path).map(Some)
} else {
Ok(None)
}
})
})
.filter_map(Result::transpose)
.collect::<Result<Vec<()>, std::io::Error>>()
// Ignore this, just an example
fn is_bundle(path: &Path) -> bool {
path.extension().map(|ext| ext == "bundle").unwrap_or(false)
}
fn is_markdown(path: &Path) -> bool {
path.extension().map(|ext| ext == "md").unwrap_or(false)
}
- Already__Taken 4y agoForgive me for asking instead of looking, I'm still getting into rust; why do you map instead of getting the result? path.extension().map(|ext| ext == "bundle").unwrap_or(false) vs (path.extension() == "bundle").unwrap_or(false) or further up .map(|e| e.map(|e| e.path())) vs .map(|e| e.path()) Is the .map keeping any value wrapped in an iterator so you can map again? I thought it was interesting what you wrote looks a lot like what I'd write in powershell.
- steveklabnik 4y agoNot your parent, but I do know the answer. extension() returns an Option<&OsStr>, because not every file has an extension https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.extension https://doc.rust-lang.org/stable/std/path/struct.Path.html#m... map on option says "run this closure on the value if it's Some, or do nothing if it's None https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.map https://doc.rust-lang.org/stable/std/option/enum.Option.html... So after the map, we now have an Option<bool>. The unwrap_or method will say "give me the value if it's Some, and give me the argument if it's not https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.unwrap_or https://doc.rust-lang.org/stable/std/option/enum.Option.html... That means we finally end up with a bool. (path.extension() == "bundle").unwrap_or(false) This code wouldn't work because you're comparing an Option<&OsStr> to a &str. Even if that worked, you'd have a boolean, and unwrap_or doesn't make sense on booleans, you'd just leave off that part.
- Already__Taken 4y agoAwesome thanks, I wasn't on a machine to play around with it at the time.
- 0x457 4y ago`extension()` gives you an `Option`, remember `Path` might not be a file or file could be without extension. Why not `((path.extension() == Some("bundle"))`? I don't know, it didn't work, and I didn't want to spend too much time on it. `read_dir` gives us an iterator over `Result<DirEntry>`, so `e` in that map is a `Result` and not a `DirEntry`. It's two different "map" functions - on Option<T> and Result<T,E> it maps "T", but on Iterator it maps "Item" from that iterator. Here is an explanation why `read_dir` give you a Result - https://doc.rust-lang.org/std/fs/fn.read_dir.html#errors https://doc.rust-lang.org/std/fs/fn.read_dir.html#errors
- Already__Taken 4y agoThanks for the insight
- throwaway894345 4y agoI'm sure there's plenty of room for improvement. I spent several hours with a bunch of Rustaceans just trying to get the short-circuiting to work right, and we sort of gave up on it beyond that.