HiveBrain v1.2.0
Get Started
← Back to all entries
patternrustMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lineprintsnumbersprogramtinythatsumcommand

Problem

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::() {
            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?

Solution

-
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) => {
| ^


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

-
Learn and love Rustfmt.

  • There are no braces on one-line match arms.



  • Always use trailing commas on multiple line items.



  • Use spaces inside braces ({ break }).



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

-
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.

-
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.

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

-
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 {
    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);
        }
    }
}

Code Snippets

#[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);
        }
    }
}

Context

StackExchange Code Review Q#150214, answer score: 4

Revisions (0)

No revisions yet.