patternrustMinor
A tiny command-line program that prints a sum of numbers
Viewed 0 times
lineprintsnumbersprogramtinythatsumcommand
Problem
Here's my first Rust program that I could actually use for something:
Did I violate any style rules? Is there anything else I should add/change/improve?
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.
-
If you don't care about the value, use an underscore (
-
Learn and love Rustfmt.
-
If the current code encounters an error while reading input, it continues processing. That seems unintuitive.
-
The current code uses
-
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
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.