12
\$\begingroup\$

I'm learning Rust using The Rust Programming Language. I'm trying the assignment at the end of chapter 8 — Hash Maps.

The task is:

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

The average value and the median were very easy for me, but I struggle solving the last part of the task. I do have a working solution (see below), but I'm sure there must be some more elegant and readable way (using the standard library?) for this.

use std::collections::HashMap;
fn main() {
  let  mut numbers = vec![42, 1, 36, 34, 76, 378, 43, 1, 43, 54, 2, 3, 43];

  let avg: f32;
  let median: i32;
  let mode: i32;

  { // calculate average
      let mut sum: i32 = 0;
      for x in &numbers {
          sum = sum + x;
      }

      avg = sum as f32 / numbers.len() as f32;
  }

  { // calculate median
      numbers.sort();
      let mid = numbers.len() / 2;

      median = numbers[mid];
  }

  { // calculate mode
      // new HashMap
      let mut times = HashMap::new();

      // count
      for x in &numbers {
          let cnt = times.entry(*x as usize).or_insert(0);
          *cnt += 1;
      }

      let mut best: (i32, i32) = (*times.iter().nth(0).expect("Fatal.").0 as i32, *times.iter().nth(0).expect("Fatal.").1 as i32);

      for x in times.iter() {
          if *x.1 > best.1 {
              best = (*x.0 as i32, *x.1);
          }
      }
      mode = best.0;
  }

  println!("AVERAGE: {}", avg);
  println!("MEDIAN: {}", median);
  println!("MODE: {}", mode)

}
\$\endgroup\$

migrated from stackoverflow.com Aug 18 '17 at 13:04

This question came from our site for professional and enthusiast programmers.

  • \$\begingroup\$ You can use iterator's sum method to get the sum, and max_by_key to find the highest value in the HashMap. \$\endgroup\$ – interjay Aug 17 '17 at 14:35
13
\$\begingroup\$
  1. Learn to love rustfmt. For example, the Rust standard is 4-space indents.

  2. Learn to love Clippy, which can show you the more idiomatic way to iterate over a collection:

    warning: it is more idiomatic to loop over references to containers instead of using explicit iteration methods
      --> src/main.rs:43:18
       |
    43 |         for x in times.iter() {
       |                  ^^^^^^^^^^^^ help: to write this more concisely, try: `&times`
       |
       = note: #[warn(explicit_iter_loop)] on by default
       = help: for further information visit https://github.com/Manishearth/rust-clippy/wiki#explicit_iter_loop
    
  3. There's no reason to declare your variables early in the function. That's the kind of thing you would do in very old C or JavaScript and isn't required in modern languages. Declare them as late as possible.

  4. Extract methods for each of the operations you'd like to perform. This allows you to remove the artifical blocks inside the main function and makes the comments into code identifiers. This removes the need for the variables in the main function at all.

  5. There's no need to use a Vec as you never add or remove items from the list. A plain array will suffice.

  6. As mentioned in the comments, Iterator::sum already exists. If it didn't, you could use something like Iterator::fold.

  7. Your implementation of median is incorrect. When there are an even number of values, the median is defined as the average of the two middle values. I've not changed your code for this because doing so would require changing the type of median and I'm not sure how you'd prefer to do so.

  8. There's no need to cast the value to usize when counting the number of occurrences; storing the i32 in the HashMap makes more sense.

  9. You can dereference the number using &value in the for loop binding.

  10. Once you have built your mapping of occurrences, you can iterate over it, taking the maximum value with Iterator::max_by_key, throw away the count using Iterator::map, then have a single expect call.

  11. Use a better error message than "Fatal" - describe what the problem was.

  12. Don't use nth(0) - that's just .next()

use std::collections::HashMap;

fn average(numbers: &[i32]) -> f32 {
    numbers.iter().sum::<i32>() as f32 / numbers.len() as f32
}

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

fn mode(numbers: &[i32]) -> i32 {
    let mut occurrences = HashMap::new();

    for &value in numbers {
        *occurrences.entry(value).or_insert(0) += 1;
    }

    occurrences
        .into_iter()
        .max_by_key(|&(_, count)| count)
        .map(|(val, _)| val)
        .expect("Cannot compute the mode of zero numbers")
}

fn main() {
    let mut numbers = [42, 1, 36, 34, 76, 378, 43, 1, 43, 54, 2, 3, 43];

    println!("AVERAGE: {}", average(&numbers));
    println!("MEDIAN: {}", median(&mut numbers));
    println!("MODE: {}", mode(&numbers));
}
\$\endgroup\$
  • \$\begingroup\$ Well, thank you for your excellent answer which is actually I bit more than I asked for in the first place, but it helped a lot, indeed. My problem was that I did not unterstand the '.map_by_key'-function, because I'm just started learning Rust and this usage of the function's parameters is completly new to me. \$\endgroup\$ – PEAR Aug 20 '17 at 12:55
  • 3
    \$\begingroup\$ @PEAR which is actually I bit more than I asked — yup, that's one of the reasons I recommended that this question be moved from Stack Overflow. \$\endgroup\$ – Shepmaster Aug 20 '17 at 15:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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