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)
} 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.extension() returns an Option<&OsStr>, because not every file has an 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...
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...
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.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
I may just have more time to dick around, but I've been forcing myself to write everything as an iterator in Rust (instead of a for loop) just to learn the paradigm and, honestly, it's been pretty great.
First, there is a performance impact. You may think this would be small, but, in my experience, it can be relatively large. There must be several reasons of which I am ignorant, but they probably include mutable no alias inside closures, bounds check elision, etc.
Second, there is a mutability advantage to just using filter/flatten/map/collect for 90% of your use cases.
Third, the more advanced iterator methods will cover the other 5-10% of your use cases, and will make your code much easier to read, once you understand how to use them (if you're trying to learning something/anything more complex at the same time, save your brain power!). I don't have a single traditional for loop in my personal projects.
My take on iterators is, for iterator/Rust beginners (like me!/perhaps not you) -- the best way to learn is to get it working as imperative code, and when you have the time, go back a rewrite as an iterator. In the beginning, it's super frustrating. But you'll get better and better and, after awhile, you'll much, much prefer writing as an iterator. You won't even think of using a for loop.
for item in iterator {
vector.push_back(foo(item));
}
You're not explicitly indexing the source iterator or the destination vector at any point."I think people make too big a deal about iterators. The simple for loops that they’re fit to replace aren’t that hard to read or write, so they aren’t causing real problem"
> I disagree. If I can avoid an index variable, I’m going to have less bugs since I won’t have to index
Which (given thread context) sounds pretty clearly like he's advocating against `for item in iterator {...}` and in favor of iterator chains. If there's another way of reading that comment, I'm not seeing it. :)
Not only that. In safe Rust vector indexes are always bound-checked that slows execution a little bit. Iterators over a vector, on the other hand, do not suffer from bound-check overhead.
However one reason to prefer an iterator chain in some cases is that the chain might imply an optimisation you'd otherwise have to write manually and might not think of.
For example if I have N things, and I map them, perhaps more than once, but I'm definitely getting N of whatever the output of the last map was, the iterator chain might very well see that I don't actually have any way to change N and so when I collect() that into a Vec the Vec it gives me is constructed with capacity N, saving the (amortized but non-zero) cost of growing it during insertions.
Imperatively I can remember to write Vec::with_capacity(N), and that's safe of course but it's an extra step to remember.
The imperative approach does particularly shine on the opposite edge of this, if I know that I'm getting no more than 100 items out of this pipeline, despite putting N in, Rust almost certainly won't see that and won't make Vec::with_capacity(100) for a collect() call whereas my imperative code can just explicitly write Vec::with_capacity(100) or whatever.
If you use "take" for example, collect should do the right thing: https://doc.rust-lang.org/src/core/iter/adapters/take.rs.htm...
It will only care about your hints if you implement the unsafe trait TrustedLen (which promises those hints are correct)
Take is indeed TrustedLen if the Iterator it's taking from is TrustedLen (as in this case it can be sure of the size hint)
If you get to ~100 via some means other than take()ing 100 of them, chances are you don't have TrustedLen as a result.
https://doc.rust-lang.org/src/alloc/vec/spec_from_iter_neste...
I wonder how the hell I didn't see that when I was tracking down how collect() ends up caring about TrustedLen.
Thanks.
I personally use iterator-style for 1-3 simple ops, but go into a loop for anything more complicated. The functional part of me wants to force iterator style but I fight it for complex use cases because it is irrational on my part, and I find loops easier to read for stuff like that.
I however will often have a dozen maps/filters/etc and that style in imperative is what, nesting repeatedly or using `continue`. Meh.
If your Iterator consists of a single `filter_map` i can see why you don't get much value out of them in that case. It's because.. well, there's not much value to get out of it.
Here's an example where the iterator-chain version is way more complex:
Here's the imperative version: