4
\$\begingroup\$

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?

\$\endgroup\$

1 Answer 1

4
\$\begingroup\$
  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);
        }
    }
}
\$\endgroup\$
5
  • 1
    \$\begingroup\$ Wow, this quick_error thing is nice! You separated error handling from the general logic in a great way, thanks. \$\endgroup\$
    – d33tah
    Dec 18, 2016 at 18:42
  • \$\begingroup\$ @d33tah I think that separation is very valuable, especially in Rust, where many more classes of errors are front-and-center. Not many other languages expose the potential for reading from stdin to fail, but it can certainly happen! \$\endgroup\$
    – Shepmaster
    Dec 18, 2016 at 18:59
  • \$\begingroup\$ I re-read the code, experimented with it and tried to actually understand it and my brain freezes at context(num: &'a str, err: std::num::ParseIntError) -> (err, num.into()). I guess I'll need to learn a bit to be able to use libraries like that. Could this code be as clean without quick_error? \$\endgroup\$
    – d33tah
    Dec 18, 2016 at 19:05
  • 1
    \$\begingroup\$ @d33tah macros can appear to be some of the more magical aspects of Rust, but usually they are "just" ways of expressing a shorthand for a common pattern. In this case, that bit of macro is likely creating a From implementation that converts into the Error type. I'd certainly encourage you to implement Error by hand to start with. Just... delete quick_error! { ... } and the call to .context(...) and see if you can reimplement the same functionality. You'll learn about std::error::Error and From / Into and maybe some more. \$\endgroup\$
    – Shepmaster
    Dec 18, 2016 at 19:20
  • \$\begingroup\$ Once you have done that, then you will have an appreciation for the shorthand that the macro gives you, or maybe you'll want to look at other libraries that provide similar convenience... \$\endgroup\$
    – Shepmaster
    Dec 18, 2016 at 19:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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