let mut idx = HashMap::new();
for fname in &args {
let f = match fs::File::open(fname) {
Ok(f) => f,
Err(e) => panic!("input file {} could not be opened: {}", fname, e),
};
let f = io::BufReader::new(f);
let mut words = HashSet::new();
for line in f.lines() {
for w in line.unwrap().split_whitespace() {
if words.insert(w.to_string()) { // new word seen
idx.entry(w.to_string()).or_insert(Vec::new()).push(fname);
}
}
}
}
(for {
fname <- args
f = fs::File::open(fname).orElse(
e => panic!("input file {} could not be opened: {}", fname, e))
r = io::BufReader::new(f)
w <- f.lines().flatMap {
line => line.unwrap().split_whitespace()
} .distinct
} yield Map(fname -> Vec(w.toString))).sum
(I thought I knew Rust, but I haven't used it for a year, and its usage has changed a lot even if the core language hasn't.)
I write C++ for most of my day job, and this would not pass code review because of readability problems in my team.
The two loops contain a single statement and it is trivial to see what's going on. It is also easier to extend. And it is still using lazy iterators (I assume) where it makes sense, i.e. in the split_whitespace call.
- The error handling should really have been refactored. `try!` would make this easier. (I haven't used it since it is in the `main` function and the code was a direct replacement.)
- Error during reading words is not accurately handled. Again, `try!` would make this easier.
- `words` and `idx` look coupled to each other, which ideally shouldn't have been.
- `words.insert` is quite an opaque method; not everyone is sure if it returns true on a duplicate key or not. I've added a comment but frankly I'm not satisfied of that. <deleted> One alternative is to use `words.entry` instead, which gives a named enum variant. </deleted> Oops, `HashSet` does not have `entry`...
- Ultimately, the body of the outermost loop should go to a function.
That said, do you really think that `line.unwrap().split_whitespace().map(|w| w.to_string()).collect::<Vec<_>>().into_iter()` is a chunk of code which can be read at a glance? It at least has to be named (like T-R's example). Functional approach means that you can split functions and individually review them; the original code, IMHO, didn't.
The nice thing about functions like "map", "filter", "fold", "flatmap", etc., is that they describe intent - when you see a "map", you know it's not aggregating things, just applying a function: "map" has a distinct purpose from "fold" (and depending on how pure things are, you may not even be allowed to do anything crazy).
Aside from that, the higher-order functions have pretty intuitive algebraic laws for refactoring (like with "compose" mentioned in my other comment) - It's not clear how you'd factor code out of a nested loop without understanding the whole thing, whereas "flatmap" (a.k.a. "bind" for the list monad) has laws for refactoring it.
> It's not clear how you'd factor code out of a nested loop without understanding the whole thing, [...]
You are right, loops are particularly hard to refactor. I still argue that my code is better (barring any future expansion) because it fits within handful lines; you cannot easily understand 100 lines of code with the cyclomatic complexity of 1, but you can often easily understand 10 lines of code with the cyclomatic complexity of 8 (e.g. triple loops). The size of code, syntax and contextual information matters as much as algebraic laws.
let mut words = HashSet::new();
for line in file.lines() {
let line_words = line.unwrap().split_whitespace();
words.extend(
line_words.map(|s| s.to_string())
);
}
words.into_iter().map(move |word| (word, f))Retrofitting better error handling in the single expression version requires changing the shape of the whole thing, editing the expression throughout.
let words = fn (f, file) => {
file.lines().flat_map( |line| {
line.unwrap()
.split_whitespace()
.map(|w| w.to_string())
.collect::<Vec<_>>()
.into_iter()
})
.collect::<HashSet<_>>() // prune duplicates
.into_iter()
.map(move |word| (word, f)) // and emit inverted index entry
}
let idx = args.iter()
.map(|fname | (fname.as_str(), fs::File::open(fname.as_str()))) //+ (str, fd)
.map(|(fname, f)| { f.and_then(|f| Ok((fname, f))).expect(&format!("input file {} could not be opened", fname)) })
.map(|(fname, f)| (fname, io::BufReader::new(f))) //+ (str, buf)
.flat_map(words) //+ {str}
// absorb all entries into a vector of file names per word
.fold(HashMap::new(), |mut idx, (word, f)| { idx
.entry(word)
.or_insert(Vec::new())
.push(f);
}); //+ {word:[filename]} let open_file = |fname| (fname.as_str(), fs::File::open(fname.as_str()) );
let check_err = |(fname, f)| {
f.and_then(|f| Ok((fname, f)))
.expect(&format!("input file {} could not be opened", fname)) };
let to_buff_reader = |(fname, f)| (fname, io::BufReader::new(f));
let line_to_words = |line| {
line.unwrap()
.split_whitespace()
.map(|w| w.to_string())
.collect::<Vec<_>>().into_iter() };
let get_file_words = |(f, file)| {
file.lines()
.flat_map(line_to_words)
.collect::<HashSet<_>>().into_iter() // prune duplicates
.map(move |word| (word, f)) }; // emit inverted index entry
let add_to_index = |mut idx, (word, f)| {
idx.entry(word).or_insert(Vec::new()).push(f);
idx };
let idx = args.iter()
.map( compose(to_buff_reader, check_err, open_file) ) //[1]
.flat_map(get_file_words)
.fold(HashMap::new(), add_to_index);
Rust itself does not have a compose function. But a closure works fine, so `|fname| to_buff_reader(check_err(open_file(fname)))` would work. If you want to go further, a nightly Rust allows for this kind of construction:
#![feature(unboxed_closures, fn_traits)] // that's why we cannot use stable (yet)
struct Composed<F, G>(pub F, pub G);
impl<F, G, T, U, V> FnOnce<T> for Composed<F, G>
where F: FnOnce<T, Output=U>, G: FnOnce<(U,), Output=V> {
type Output = V;
extern "rust-call" fn call_once(self, args: T) -> V { self.1(self.0.call_once(args)) }
}
impl<F, G, T, U, V> FnMut<T> for Composed<F, G>
where F: FnMut<T, Output=U>, G: FnMut<(U,), Output=V> {
extern "rust-call" fn call_mut(&mut self, args: T) -> V { self.1(self.0.call_mut(args)) }
}
impl<F, G, T, U, V> Fn<T> for Composed<F, G>
where F: Fn<T, Output=U>, G: Fn<(U,), Output=V> {
extern "rust-call" fn call(&self, args: T) -> V { self.1(self.0.call(args)) }
}
fn main() {
println!("{}", Composed(|x| x+3, |y| y*4)(5)); // 32
}Ah, yes it would. Clearly I'm a bit too tired to be writing code, if I've overlooked function application. I suppose it doesn't have to be point-free. =)
Pretty cool, though - thanks for the info.
However, its overloading a custom type to behave like a higher order function, which is a fairly complex piece of code. Most uses of generics are much simpler.
Some people seem to fall back into it a bit when they discover point-free style (and that seems to make up a lot of what you see in mixed-paradigm code). I don't think it'd be controversial to say it's bad practice in any paradigm, so the association of it with FP is kind of like judging web programming by late 90's beginner PHP code (which, at one point in time, did describe a lot of web programming, but it was never good, and we'd like to put that behind us).
I opened the external example he linked which has solutions in a number of languages. (https://www.rosettacode.org/wiki/Inverted_index) I could understand the C version of the code pretty well at a glance. Granted it was long but it looks like the intuitive way I'd write the code. I acknowledge my bias here.
I looked at some other languages on the site and both the the C# example and D example look much more readable then the rust code (and I've not used either of those languages either I think they are intended for "system" programming as well) the code as provided in those languages seemed more intuitive.
That said, there's certainly a point to be made (and indeed other commenters have made it already) that this isn't code you would want to maintain. For production-level code, it'd be broken down more, so that individual pieces could be reviewed and tested in isolation. The code example here was to demonstrate the expressivity of the language more so than the One True Way of doing it.
This looks like code from the 'I found out about FP, let's apply it everywhere'-stage. For instance, if the iteration over arguments was done with a regular for...in loop, the filename would be visible in its scope and the rest of the code could be simplified due to not having to pass the filename everywhere.
Also, I don't think this demonstrates expressivity well, because the purported functional construct uses unwrap/expect (it panics). The expressivity of Rust allows you to have the whole expression to have Result has its type fairly easily.
(Sorry for ranting a bit, but I think that Not only is this very readable, will tick off people. Which is sad, because it could be changed into something which is more readable but still expressive.)
The functional code I give in the article is quite radically different from what you would/could do in those languages, and I would argue that it does show that Rust provides some interesting and flexible mechanisms that add to the expressivity you have as a programmer. Sure, it's overused in the example, but the exaggeration at least has a chance of communicating the point. Maybe the argument is that I should have used a simpler (yet still not trivial) example instead, but I couldn't think of one at the time.
Re the "Not only is this very readable" part, I completely agree. I could change it to something less, hmm, bold, but I'm not sure it would make that much of a difference at this point. Suggestions are welcome.
It went thought the code identifying code patterns and goals, and then making them into separate functions or data structures as appropriate.
Rust does not force you into the functional style, and usually I use imperative style when writing Rust code, except when the task is so simple that writing e.g. a for loop would be just noise. For example, if I just wanted to convert every string in a vector to uppercase, I'd probably just write "list.iter(|string| string.to_uppercase()).collect()". But if I were, say, hashing every string in the vector, I'd probably use a for loop over an iterator.
There are enough similarities with ML languages, and I have used so much FP concepts all the way back to Caml Light, my first FP language, that I would just naturally follow that path when writing Rust.
In Smalltalk it was just natural to mix both styles.
I use the same strategy in "dynamic" languages like Python and JavaScript, that support both an imperative and functional style.
https://gist.github.com/danieldk/3bd3b84c1a7c8bc8c902314a488...
(Compiles and seems to work properly on a small document collection.)
> .iter()
hmmm. Not far off the old
> i++ // add one to i