Rust: calculating mean, median, and mode for numbers from stdin

Question

This quiz is on The Rust Programming Language online ebook.

Given a list of integers, use a vector and return the mean (the average value), median (when sorted, the value in the middle position), and mode (the value that occurs most often; a hash map will be helpful here) of the list.

As a novice Rust programmer, I'd like get my code reviewed and learn more about programming Rust.

There are a couple of things I'd like to know:

• getting input using io::stdin().read_line() is ok for this case,
• if there's better looping (user-input) and quitting pattern,
• handling user input errors with helpful error message (Your input is invalid. Please enter whitespace-separated integers.) then re-ask with prompt, instead of panicking,
• whether type casting using as is commonly-used (ex: i32 -> f32),
• implementation of get_median() and get_mode() can be simpler than now

Here's my code:

use std::collections::HashMap;
use std::io::{self, prelude::*};

fn main() {
    let stdin = io::stdin();

    'mainloop: loop {
        print!("Enter space-separated numbers (q for quit)> ");
        io::stdout().flush().expect("flush failed");

        let mut line = String::new();
        stdin.read_line(&mut line).expect("Failed to read line");

        if line == "q\n" {
            break 'mainloop;
        }

        // TODO: handling ParseIntError
        let nums: Vec<i32> = line
            .split_whitespace()
            .map(|s| s.parse::<i32>().unwrap())
            .collect();

        let mean = nums.iter().sum::<i32>() as f32 / nums.len() as f32;

        println!(
            "numbers: {:?}, mean: {}, median: {}, mode: {}",
            nums,
            mean,
            get_median(&nums),
            get_mode(&nums)
        );
    }
}

fn get_median(v: &Vec<i32>) -> f32 {
    if v.len() < 1 {
        return 0.0;
    }

    let mut vec = v.clone();
    vec.sort();
    if vec.len() % 2 == 1 {
        return *vec.get(vec.len() / 2).unwrap() as f32;
    }
    return (*vec.get(vec.len() / 2 - 1).unwrap() + *vec.get(vec.len() / 2).unwrap()) as f32 / 2.0;
}

fn get_mode(v: &Vec<i32>) -> i32 {
    let mut map = HashMap::new();
    for num in v {
        let count = map.entry(num).or_insert(0);
        *count += 1;
    }
    return **map.iter().max_by_key(|(_, v)| *v).unwrap().0;
}

• This code is on GitHub also.
• I found there are similar questions related to the same quiz on Code Review, but the implementations differ. So I think they're not duplicated question.

Answer 1

Good job! I'm sure there are problems with my take, but, for what it's worth, here's how I might suggest improving things a bit.

use std::collections::HashMap;
use std::io::{self, Write}; // Personal preference, but I prefer avoiding glob imports
use std::num::ParseIntError;

fn main() {
    let stdin = io::stdin();
    let mut stdout = io::stdout();

    // Allocate a String buffer once and keep reusing it, instead of allocating multiple times.
    let mut line = String::new();

    // No need to name the loop since it's the only one. Loop naming
    // isn't very idiomatic in rust; it's typically used only when there
    // are loops within loops and naming is needed to solve ambiguity.
    loop {
        print!("Enter space-separated numbers (q for quit)> ");
        // Unwrap here seems OK, since if flush() fails, something went terribly wrong.
        stdout.flush().unwrap();

        // Clear the String buffer to prepare for reuse.
        line.clear();
        // Unwrap here seems OK, for similar reasoning as above.
        stdin.read_line(&mut line).unwrap();

        if line == "q\n" {
            break;
        }

        // Let the closure passed to map return a Result, then collect into a
        // Result<Vec<_>, ParseIntError>, which will be Ok if all parsing succeeded
        // or Err if any parsing failed. After that, match on your Result to handle errors.
        let mut nums = match line
            .split_whitespace()
            .map(|s| s.parse::<i32>())
            .collect::<Result<Vec<i32>, ParseIntError>>()
        {
            Ok(vec) => {
                if vec.is_empty() {
                    // Error handling: If user didn't give you any input, try again.
                    continue;
                }
                vec
            }
            Err(_) => {
                // Error handling: If user gave you bad input, try again.
                println!("Invalid input. Only integer values are accepted. Please try again.");
                continue;
            }
        };

        // Casting i32 to f32 is fine.
        let mean = nums.iter().sum::<i32>() as f32 / nums.len() as f32;
        let median = get_median(&mut nums);
        let mode = get_mode(&nums);

        println!(
            "numbers: {:?}, mean: {}, median: {}, mode: {:?}",
            &nums, // No need to consume nums (even though in this case you never use it again)
            mean,
            median,
            mode,
        );

        break;
    }
}

// Avoid the clone by passing a mutable reference
fn get_median(vec: &mut Vec<i32>) -> f32 {
    // is_empty() is slightly more idiomtic than vec.len() < 1
    if vec.is_empty() {
        return 0.0;
    }

    vec.sort();

    // * `vec.len() / 2` is reusable code; so make it it's own varible
    // * Instead of using `vec.get(index).unwrap()`, use `vec[index]`,
    //   as we know our index is valid.
    let index = vec.len() / 2;

    if vec.len() % 2 == 1 {
        vec[index] as f32
    } else {
        (vec[index - 1] as f32 + vec[index] as f32) / 2.0
    }
}

// * Whenever you want to pass &Vec<_> as an argument to a function,
//   consider passing &[_] instead. It works with more types.
// * Mode is not necessarily unique; so return a Vec<i32>.
fn get_mode(slice: &[i32]) -> Vec<i32> {
    if slice.is_empty() {
        // bail early if the input is empty; so that we avoid
        // unnecessarily allocating a HashMap.
        return vec![];
    }

    // Waste a little space by allocating from the start a HashMap
    // of maximum possible size; this avoids the possibility of
    // having to rellocate once the size has grown bigger than
    // capacity. It's a micro optimization, but in general in rust
    // if you know how big a heap allocated thing (like Vec, String, HashMap)
    // etc. needs to be in advance; take advantage of that information.
    let mut map = HashMap::with_capacity(slice.len());
    for num in slice {
        let count = map.entry(num).or_insert(0);
        *count += 1;
    }

    // Unwrap is OK because we know the map is non-empty.
    let max_value = map.values().map(|v| *v).max().unwrap();

    // Use into_iter() as we don't need the map anymore and
    // that will more efficiently create the vec than
    // if we had used iter()
    let mut vec = map
        .into_iter()
        .filter_map(|(k, v)| if v == max_value { Some(*k) } else { None })
        .collect::<Vec<i32>>();

    // Sort because it makes the output deterministic
    vec.sort();

    vec
}
```