The Book chapter 08 exercise 1: Musings on Hashmaps

code review
1

Hi,

currently I am working through the Rust Programming Language book and am stuck in chapter 08 at exercise 1. I put the exercise text in the code comment below.

My question is: is my code as ugly as it feels to me (see: // ugly... comment)? I am certain that there are better solutions to this ... but are there better solutions if you only take the book up to chapter 08 into account? ... I am wondering what possible solution the author of the exercise had in mind.

Thanks for any hints. :slight_smile:

// Given a list of integers, use a vector and return the
// 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.

use std::collections::HashMap;

fn median(mut numbers: Vec<i32>) -> i32 {
    let index = numbers.len() / 2;
    numbers.sort();
    numbers[index]
}

fn mode(numbers: Vec<i32>) -> Option<i32> {
    if numbers.is_empty() {
        return None;
    }

    let mut map = HashMap::new();
    for n in numbers {
        let count = map.entry(n).or_insert(0);
        *count += 1;
    }

    // ugly ...
    let mut out = 0;
    let mut max = 0;
    for (k, v) in map {
        if v > max {
            max = v;
            out = k;
        }
    }
    Some(out)
}

fn main() {
    let numbers = vec![-1, 42, -10, 2, -23, 12, -3, 42, -5, 23, -8, 16, -42, 13];
    println!("Median: {}", median(numbers.clone()));
    println!("Mode: {}", mode(numbers).expect("Error"));
}

(Playground)

Output:

Median: 2
Mode: 42

Errors:

   Compiling playground v0.0.1 (/playground)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 1.03s
     Running `target/debug/playground`
2

You could use iterator methods to replace your loop.

There may be a difference when more than one value has the same count (I didn't check). I also don't know what behavior was expected for that case.

1 Like
3

You can also use max_by_key:

map.into_iter().max_by_key(|(_, count)| *count).map(|(val, _)| val)

2 Likes
4

Hi,

thanks for the suggestions. Sadly these features (iterators and these other things I cannot even name :D) have not been introduced in chapter 08, so I feel I should not use them for this exercise. :slight_smile:

5

It looks fine to me.

1 Like
6

I don’t know the exact exercise, but commonly the median of an odd-sized series is the middle element, while for an even-sized series it is the mean of the two middle elements.

1 Like
7

Meh. While that's commonly stated, it's an arbitrary hybrid between "median" and "mean". I think returning one of the two middle elements is a better definition. Taking the mean would require returning a fraction here. Returning one of the two also allows you to return a reference, and it works for all Ord types rather than just for numbers.

2 Likes
8

Thank you all for your input. I think it is fair to say, that you all are beyond chapter 8 and so I will be soon. So I guess it is fine for me to write code I do not like as long as I know how to write code I like better in the future. :slight_smile:

1 Like