Rust Simple Stats Program

Question

I am brand new to Rust and want to know if there are any convention violations in my code and ways how I can make it more readable:

use std::io;

fn main() {
    let mut vec: Vec<i32> = Vec::new();
    loop {
        println!("Enter any number, 0 to terminate");
        let mut num = String::new();
        io::stdin()
            .read_line(&mut num);
        if num.trim().parse::<i32>().unwrap() == 0 {break;}
        vec.push(num.trim().parse::<i32>().unwrap());
    }
    let sum: i32 = vec.iter().sum();
    println!("Sum: {}", sum);
    println!("Min: {}", vec.iter().min().unwrap());
    println!("Max: {}", vec.iter().max().unwrap());
    println!("Mean: {}", sum as f32 / vec.len() as f32);
}

Answer 1

io::stdin().read_line(&mut num);

Here you are ignoring the Result from read_line; you should have seen a compiler warning about this. Add .unwrap() or .expect() to check the error and panic if there is a problem reading:

io::stdin().read_line(&mut num).expect("Read error");

(Below I'll be talking about handling errors instead of panicking. This case is different because an IO error during reading shouldn't occur in normal usage and there isn't really any way to recover from it.)

---

        if num.trim().parse::<i32>().unwrap() == 0 {break;}
        vec.push(num.trim().parse::<i32>().unwrap());

In these two lines you are parsing the number twice. Put the parse result in a variable instead.

        let num = num.trim().parse::<i32>().unwrap();
        if num == 0 {break;}
        vec.push(num);

---

If the user makes a typo and enters something that does not parse as i32 (e.g. a letter) then the program will panic since you are .unwrap()ing the parse result. It probably makes sense to tell the user and let them reenter instead. We can combine that with the handling of 0 inputs:

        match num.trim().parse::<i32>() {
            Err(_) => {
                println!("{:?} wasn't an integer. Try again.", num);
            }
            Ok(0) => {
                break;
            }
            Ok(x) => {
                vec.push(x);
            }
        }

---

Finally, you used .unwrap() for the min and max, which means that it will panic if the user doesn't enter any numbers. It's generally better for a program to not panic on any input. We can eliminate the unwrap from a single print like this:

    if let Some(min) = vec.iter().min() {
        println!("Min: {}", min);
    }

But, since three of the four outputs don't make sense for zero items, I think it makes more sense to put them inside a single condition, even if that means keeping the unwraps:

    println!("Sum: {}", sum);
    if !vec.is_empty() {
        println!("Min: {}", vec.iter().min().unwrap());
        println!("Max: {}", vec.iter().max().unwrap());
        println!("Mean: {}", sum as f32 / vec.len() as f32);
    }

---

One more thing. If the input to the program reaches end-of-file without containing 0, it will loop infinitely, reading no characters and producing error messages. (Well, your initial version didn't, because it panicked on a parse error instead). The right thing to do is to check whether read_line met end of file; this is reported in the return value but also detectable by the fact that num will be the empty string (without any newline).

        io::stdin().read_line(&mut num).unwrap();
        if num == "" {
            break;
        }

---

Putting together all my suggested changes:

use std::io;

fn main() {
    let mut vec: Vec<i32> = Vec::new();
    loop {
        println!("Enter any number, 0 to terminate");
        let mut num = String::new();
        io::stdin().read_line(&mut num).unwrap();
        if num == "" {
            break;
        }
        match num.trim().parse::<i32>() {
            Err(_) => {
                println!("{:?} wasn't an integer. Try again.", num);
            }
            Ok(0) => {
                break;
            }
            Ok(x) => {
                vec.push(x);
            }
        }
    }
    let sum: i32 = vec.iter().sum();
    println!("Sum: {}", sum);
    if !vec.is_empty() {
        println!("Min: {}", vec.iter().min().unwrap());
        println!("Max: {}", vec.iter().max().unwrap());
        println!("Mean: {}", sum as f32 / vec.len() as f32);
    }
}

Answer 2

1. If you really intended the app to be used, you would probably want to use StructOpt to give help, etc.
2. There are several places where the program looks like it would crash if you just immediately gave it a 0 without any other values: the .unwrap()s and the division at the end. You would probably want to check for that up front.
3. Rather than doing unwrap()s like that you could use the anyhow crate and make main return a Result<()>. Then your .unwrap()s could just be ?s.
4. You can change the way input is provided to make the program much more flexible AND make the code easier to read. Just read lines from stdin until EOF, flat_map them to the numbers they contain, and collect the results as a Vec -- all in one line (something like stdin.lock().lines().flat_map(...).collect()).
5. You could make the program able to handle arbitrarily large data sets by NOT collecting the results in a Vec, but rather fold()ing to get the sum, min, max, and count, and then using those to produce the output at the end.

Update: I decided to go ahead and implement 2/4/5, to see what it would look like; here it is in the playground. And actually, that unwrap() on the parse should really be an expect.