A tiny command-line program that prints a sum of numbers

Question

Here's my first Rust program that I could actually use for something:

use std::io;
use std::process;

fn main() {
    let mut sum = 0;
    loop {
        let mut number_str = String::new();
        match io::stdin().read_line(&mut number_str) {
            Ok(n) => { if n == 0 {break} },
            Err(e) => { println!("ERROR: got '{}' when reading a line", e) }
        }
        match number_str.trim().parse::<i32>() {
            Ok(n) => { sum += n }
            Err(n) => {
                println!("ERROR: Entered something that is not a number: '{}'",
                    number_str.trim_right());
                process::exit(1)
            },
        }
    }
    println!("{}", sum);
}

Did I violate any style rules? Is there anything else I should add/change/improve?

Answer 1

1. You have a compiler warning. One of the points of using a statically typed language is to have the compiler watching your back. If you ignore what the compiler says, you lose out.

   warning: unused variable: `n`, #[warn(unused_variables)] on by default
     --> main.rs:14:17
      |
   14 |             Err(n) => {
      |                 ^
   

2. If you don't care about the value, use an underscore (_).

3. Learn and love Rustfmt.

   1. There are no braces on one-line match arms.
   2. Always use trailing commas on multiple line items.
   3. Use spaces inside braces ({ break }).

4. If the current code encounters an error while reading input, it continues processing. That seems unintuitive.

5. The current code uses trim when parsing, but only trim_right when showing the error to the user. This seems like a way of confusing the user when the error occurs.

6. It's potentially inefficient to reallocate the string in each iteration. Moving it outside the loop would mean you are responsible for clearing it each iteration. Failing to clear it would be a bug and potential security hole.

7. However, the code to process each line is already implemented for you.

8. There's duplicate error handling code. I'd extract a method to be able to use Result. I love using quick_error to construct error enumerations. This lets you use try! or ?.

#[macro_use]
extern crate quick_error;

use std::io::{self, BufRead};
use std::process;

use quick_error::ResultExt;

quick_error! {
    #[derive(Debug)]
    enum Error {
        Io(err: io::Error) {
            from()
            description("unable to read line")
            display("Unable to read line: {}", err)
            cause(err)
        }
        Parse(err: std::num::ParseIntError, num: String) {
            description("unable to parse number")
            display("Unable to parse '{}': {}", num, err)
            cause(err)
            context(num: &'a str, err: std::num::ParseIntError) -> (err, num.into())
        }
    }
}

fn inner_main() -> Result<i32, Error> {
    let mut sum = 0;

    let stdin = io::stdin();
    for line in stdin.lock().lines() {
        let line = line?;
        let line = line.trim();
        sum += line.parse().context(line)?;
    }

    Ok(sum)
}

fn main() {
    match inner_main() {
        Ok(sum) => println!("{}", sum),
        Err(e) => {
            println!("ERROR: {}", e);
            process::exit(1);
        }
    }
}