2
\$\begingroup\$

I am learning Rust and I would like to get some feedback around my solution to calculate the median value from a list of integers.

I believe the logic is correct as my tests pass, but I'm concerned the right types and if I coded "the Rust way".

use std::f32;

fn main() {
    let mut v = Vec::new();
    v.push(5);
    v.push(6);
    v.push(7);
    v.push(8);
    v.push(9);

    println!("{:?}", get_median(v)); // 7
    
    v = Vec::new();
    v.push(5);
    v.push(6);
    v.push(7);
    v.push(8);
    v.push(9);
    v.push(10);
    println!("{:?}", get_median(v)); // 7.5
    
    v = Vec::new();
    v.push(6);
    v.push(5);
    v.push(10);
    v.push(7); 
    v.push(9);
    v.push(8);
    println!("{:?}", get_median(v)); // 7.5
    
    v = Vec::new();
    v.push(-2);
    v.push(-9);
    v.push(-4);
    v.push(-1); 
    v.push(0);
    v.push(6);
    println!("{:?}", get_median(v)); // -1.5
}

fn get_median(numbers: Vec<i32>) -> f32 {
    let mut v = numbers;
    v.sort();
    let length = v.len() as f32;

    if length%2.0 == 1.0 {
        let index = length as usize / 2;
        v[index] as f32
    } else {
        let index = length as usize / 2;
        let lower_mid: usize = index - 1;
        let higher_mid: usize = index;

        (v[lower_mid] + v[higher_mid]) as f32 / 2.0 
    }
}
\$\endgroup\$
1
  • 2
    \$\begingroup\$ You also have to consider the case where the vector is empty. \$\endgroup\$
    – G. Sliepen
    Commented Oct 20, 2022 at 17:48

1 Answer 1

2
\$\begingroup\$

Some basics

  1. You don't need to bring f32 into scope, it's in there by default.
  2. Use rustfmt on your code.
  3. Use a linter like clippy.

Use initializer functions

You don't need to push your elements on a Vec one by one. Use Vec::from(). Or, even better, use the vec! macro.

Use a cargo project

Develop your project as a cargo crate. It will make the organization of tests and possible dependencies much easier.

Use unit tests

Use cargo's test suite to assert that your code is working.

Useless conversions

You convert Vec::len() from usize to f32 just to convert it back to usize again.

Write generic code

You can generalize your code by using generics and appropriate traits, so that it accepts other numeric iterables.

Remember edge cases

As pointed out by @G.Sliepen, you might also want to consider the case, where the vector is empty.

Suggested:

Cargo.toml:

[package]
name = "median"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

[dev-dependencies]
once_cell = "*"

src/lib.rs:

use std::ops::Add;

pub fn median<T>(items: &[T]) -> f64
where
    T: Add<Output = T> + Copy + Into<f64> + Ord,
{
    let mut items: Vec<T> = items.to_vec();

    if items.is_empty() {
        return 0.0;
    }

    items.sort();
    let len = items.len();

    if len % 2 == 0 {
        (items[len / 2] + items[len / 2 - 1]).into() / 2.0
    } else {
        items[len / 2].into()
    }
}

tests/test.rs:

use median::median;
use once_cell::sync::Lazy;

static MEDIANS: Lazy<[(f64, Vec<i32>); 5]> = Lazy::new(|| {
    [
        (7.0, vec![5, 6, 7, 8, 9]),
        (7.5, vec![5, 6, 7, 8, 9, 10]),
        (7.5, vec![6, 5, 10, 7, 9, 8]),
        (-1.5, vec![-2, -9, -4, -1, 0, 6]),
        (0.0, vec![]),
    ]
});

#[test]
fn test_median() {
    for (nominal, numbers) in MEDIANS.iter() {
        assert_eq!(*nominal, median(numbers));
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.