Calculate mode in Rust

Question

I'm new to Rust and still getting used to the ownership model. I'm done with chapter 8 of the book and was trying to solve this exercise:

Given a list of integers, use a vector and return the [...] mode (the value that occurs most often; a hash map will be helpful here) of the list.

Here's my solution:

use std::collections::HashMap;

pub fn vectors() {
    let mut numbers: Vec<i32> = vec![
        12, 15, 6, 44, 6, 32, 77, 77, 77, 77, 77, 94, 6, 2, 67, 0, 25, 14, 84, 81, 59, 31, 7, 9,
    ];

    println!("{:?}", numbers);

    // calculating and displaying the mode
    let mut numbers_count = HashMap::new();
    for num in &numbers {
        let count = numbers_count.entry(num).or_insert(0);
        *count += 1;
    }
    let mut mode = 0;
    let mut prev_value = 0;
    for (key, value) in &numbers_count {
        if *value > prev_value {
            prev_value = *value;
            mode = **key;
        }
    }
    println!("Mode: {}", mode);
}

The code works but... is this considered a good solution? After I finished I searched online and found this alternative which seems to be more accepted. Also, on line 21 I had to use the dereference operator twice; is this ok, or is it considered bad practice?

Answer 1

that looks like some really nice code, there is a few things I want to highlight, though:

1. numbers does not need to be declared mutable (and thus should not be). Always declare with the least powerful borrowing specification possible. That simplifies the compiler's job and makes the checker more valuable by exposing unintended mutability.

2. numbers and numbers_count are not really stellar names, but then again the code is small, and very clear, so that should be fine.

3. There is a much more powerful way of expressing the "get the key associated with the maximum value", using Iterator::max_by, which condenses the whole map iteration and two variables to:

   let (mode, _) = numbers_count.iter()
       .max_by(|(_, lv), (_, rv)| lv.cmp(rv))
       .expect("We need at least one value to calculate a mode");
   return **mode;
   

   Note that this update introduces a subtle semantic difference in that the last element in the iter() with the most items in the list is given instead of the first. But since the iter() returned by a HashMap is not ordered in the first place, that should not make any difference :)

4. As it is, with this solution you are not learning about one of the most powerful features of the borrow checker: Argument passing to functions.
   Instead of just working off a constant vector, it's much more enlightening to pass the vector as an argument to vectors

   pub fn mode_v1(numbers: &Vec<i32>) -> i32 {
       let mut numbers_count = HashMap::new();
       for num in numbers {
           let count = numbers_count.entry(num).or_insert(0);
           *count += 1;
       }
   ...
   

   Note how this moves the borrow behaviour enforcement to the caller of the method. This can of course be further extended to cover cases like there not being elements in the vector, but not wanting to panic when determining the mode.

   Another interesting step is to move away from the type bound of Vec<i32> to something a little more generic, by accepting more collection-y things.
   For that rust has Slice which can be easily introduced as follows:

   pub fn mode(numbers: &[i32]) -> Option<i32> { // or i32, depending on error behaviour
   

   This in turn can then be extended to include generics to allow for a more diverse set of inputs. Maybe you want the mode of a list of i64s or i8 or something else entirely?:

   pub fn mode<T>(elements: &[T]) -> Option<T>
       where T: Ord + Hash + Copy {