patternrustMinor
Moving average in Rust
Viewed 0 times
rustmovingaverage
Problem
I worked on implementing a simple moving average function in Rust, that works on a collection of year/revenue tuples:
Output:
fn compute_moving_average(window: &Box) -> (u32, f32) {
let window_size = window.len();
let current_year = window
.iter()
.nth((window_size as f32/ 2.0).floor() as usize)
.unwrap().0;
let sum = window
.iter()
.fold(0, |a, x| a + x.1) as f32 / window_size as f32;
(current_year, sum)
}
fn extract_moving_average_for_year(year: u32, moving_average: &Vec) -> Option {
let x = moving_average
.iter()
.find(|a| a.0 == year);
match x {
Some(a) => Some(a.1),
None => None,
}
}
fn merge_moving_average(a: (u32, u32), avg: Option) -> (u32, u32, Option) {
(a.0, a.1, avg)
}
fn main() {
let vec = vec![
(2003, 4),
(2004, 6),
(2005, 5),
(2006, 8),
(2007, 9),
(2008, 5),
(2009, 4),
(2010, 3),
(2011, 7),
(2012, 8),
];
let moving_average = vec
.windows(5)
.map(|a| compute_moving_average(&Box::new(a)))
.collect::>();
vec
.iter()
.map(|a| merge_moving_average(*a, extract_moving_average_for_year(a.0, &moving_average)))
.inspect(|a| println!("{:?}", a))
.collect::>();
}Output:
(2003, 4, None)
(2004, 6, None)
(2005, 5, Some(6.4))
(2006, 8, Some(6.6))
(2007, 9, Some(6.2))
(2008, 5, Some(5.8))
(2009, 4, Some(5.6))
(2010, 3, Some(5.4))
(2011, 7, None)
(2012, 8, None)
Solution
- I don't know why
compute_moving_averagetakes aBox, much less why it would take a reference to one.
- Prefer
&[T]over&Vec.
- Why use floating point division with truncation when integer math does same, should be faster, and is shorter to write?
- Don't use an iterator on a slice if you are just going to pull out the
nthvalue; just index. Also bakes-in theunwrap.
- I'd use
Iterator::mapandIterator::sum.
- Preserve the sum as an integer until the last moment, then convert it to floating point. This may or may not be a good idea depending on the input data.
- Learn and love all the methods on
OptionandResult. For example,Option::mapreplaces the 4 lines of an explicitmatch.
-
merge_moving_average seems more trouble than it's worth; just inline it and provide better variable names.-
There's no need to create a
Vec; just pass in a slice of an array.-
Can directly pass
compute_moving_average with map, no closure or arguments needed.-
It's more idiomatic to use
|&a| in the closure argument instead of *a in the body when they are equivalent.Iterator::inspectis more of a debugging tool than anything; use aforloop to drive an iterator for side effects.
fn compute_moving_average(window: &[(u32, u32)]) -> (u32, f32) {
let window_size = window.len();
let current_year = window[window_size / 2].0;
let sum: u32 = window.iter().map(|&(_, val)| val).sum();
let sum = sum as f32 / window_size as f32;
(current_year, sum)
}
fn extract_moving_average_for_year(year: u32, moving_average: &[(u32, f32)]) -> Option {
moving_average
.iter()
.find(|(yr, _)| yr == year)
.map(|&(_, val)| val)
}
fn main() {
let vec = [
(2003, 4),
(2004, 6),
(2005, 5),
(2006, 8),
(2007, 9),
(2008, 5),
(2009, 4),
(2010, 3),
(2011, 7),
(2012, 8),
];
let moving_average = vec
.windows(5)
.map(compute_moving_average)
.collect::>();
let moving_average = vec
.iter()
.map(|&(year, val)| (year, val, extract_moving_average_for_year(year, &moving_average)));
for a in moving_average {
println!("{:?}", a);
}
}Thoughts:
It would be interesting to see an implementation that doesn't use
windows, but instead is based on an iterator.If you know your slice is sorted, a binary search might be more efficient.
Code Snippets
fn compute_moving_average(window: &[(u32, u32)]) -> (u32, f32) {
let window_size = window.len();
let current_year = window[window_size / 2].0;
let sum: u32 = window.iter().map(|&(_, val)| val).sum();
let sum = sum as f32 / window_size as f32;
(current_year, sum)
}
fn extract_moving_average_for_year(year: u32, moving_average: &[(u32, f32)]) -> Option<f32> {
moving_average
.iter()
.find(|(yr, _)| yr == year)
.map(|&(_, val)| val)
}
fn main() {
let vec = [
(2003, 4),
(2004, 6),
(2005, 5),
(2006, 8),
(2007, 9),
(2008, 5),
(2009, 4),
(2010, 3),
(2011, 7),
(2012, 8),
];
let moving_average = vec
.windows(5)
.map(compute_moving_average)
.collect::<Vec<_>>();
let moving_average = vec
.iter()
.map(|&(year, val)| (year, val, extract_moving_average_for_year(year, &moving_average)));
for a in moving_average {
println!("{:?}", a);
}
}Context
StackExchange Code Review Q#149951, answer score: 2
Revisions (0)
No revisions yet.