Parsing a u8 separated vector of FromStr from a BufRead

Question

I need to read a vector of e.g. integers from a stream, usually from stdin but sometimes also from a file. The input is always less than a megabyte, and is sometimes separated by commas and sometimes by newlines.

I'm calling the code directly from my main function, so I'm more concerned with returning a human readable error rather than something that can be caught and recovered from programmatically, hence I'm using String as the error type.

pub fn parse_vector<T: BufRead, S: FromStr>(buf: T, sep: u8) -> Result<Vec<S>, String> {
 buf.split(sep)
 .enumerate()
 .filter_map(|(i, entry)| {
 let entry_nr = i + 1;
 let entry = match entry.map(String::from_utf8) {
 Err(e) => return Err(format!("Cannot read entry {}, {}.", entry_nr, e)).into(),
 Ok(Err(e)) => return Err(format!("Cannot read entry {}, {}.", entry_nr, e)).into(),
 Ok(Ok(v)) => v,
 };
 let trimmed = entry.trim();
 if trimmed.is_empty() {
 None
 } else {
 Some(
 trimmed
 .parse::<S>()
 .map_err(|_| format!("Cannot parse entry {}: '{}'", entry_nr, entry)),
 )
 }
 })
 .collect()
}

The above code works as far as I can tell, but I'm wondering if there's a nicer way to deal with the mess in the match expression. The functions from_utf8 and split return different error types, but both implement Display so maybe there's some elegant way to eliminate some code duplication there that I don't know about (I'm new to rust, coming from c++).

• What are my options to make this code more readable?
• Is it considered bad practice to return String as an error type?

Answer 1

I'm calling the code directly from my main function, so I'm more concerned with returning a human readable error rather than something that can be caught and recovered from programmatically, hence I'm using String as the error type.

[...]

• Is it considered bad practice to return String as an error type?

Yes, it is generally considered bad practice to return Result<_, String> when the error returned is formatted from other errors. The recommended approach is to define an enum and implement From, Display, and Error: (using the thiserror crate for simplicity)

use thiserror::Error;
#[derive(Debug, Error)]
pub enum Error {
 #[error("cannot read entry {entry_no}")]
 Io { entry_no: usize, source: std::io::Error },
 #[error("cannot read entry {entry_no}")]
 Encoding { entry_no: usize, source: std::string::FromUtf8Error },
 // ...
}

The above code works as far as I can tell, but I'm wondering if there's a nicer way to deal with the mess in the match expression. The functions from_utf8 and split return different error types, but both implement Display so maybe there's some elegant way to eliminate some code duplication there that I don't know about (I'm new to rust, coming from C++).

Your options include:

• methods on Option and Result, like Result::and_then and Option::ok_or_else;

• the ? operator; and

• using .zip(1..) instead of .enumerate() to get rid of that + 1,

plus potentially other things that didn't immediately come to mind.