Moving average in Rust

Question

I worked on implementing a simple moving average function in Rust, that works on a collection of year/revenue tuples:

fn compute_moving_average(window: &Box<&[(u32, u32)]>) -> (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<(u32, f32)>) -> Option<f32> {
    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<f32>) -> (u32, u32, Option<f32>) {
    (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<_>>();

    vec
        .iter()
        .map(|a| merge_moving_average(*a, extract_moving_average_for_year(a.0, &moving_average)))
        .inspect(|a| println!("{:?}", a))
        .collect::<Vec<_>>();
}

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)

Answer 1

1. I don't know why compute_moving_average takes a Box, much less why it would take a reference to one.
2. Prefer &[T] over &Vec<T>.
3. Why use floating point division with truncation when integer math does same, should be faster, and is shorter to write?
4. Don't use an iterator on a slice if you are just going to pull out the nth value; just index. Also bakes-in the unwrap.
5. I'd use Iterator::map and Iterator::sum.
6. 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.
7. Learn and love all the methods on Option and Result. For example, Option::map replaces the 4 lines of an explicit match.

8. merge_moving_average seems more trouble than it's worth; just inline it and provide better variable names.

9. There's no need to create a Vec; just pass in a slice of an array.

10. Can directly pass compute_moving_average with map, no closure or arguments needed.

11. It's more idiomatic to use |&a| in the closure argument instead of *a in the body when they are equivalent.

12. Iterator::inspect is more of a debugging tool than anything; use a for loop 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<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);
    }
}

---

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.