51

I want to read a file and get back a vector of Strings. The following function works, but is there a more concise or idiomatic way?

use std::fs::File;
use std::io::Read;

fn lines_from_file(filename: &str) -> Vec<String> {
    let mut file = match File::open(filename) {
        Ok(file) => file,
        Err(_) => panic!("no such file"),
    };
    let mut file_contents = String::new();
    file.read_to_string(&mut file_contents)
        .ok()
        .expect("failed to read!");
    let lines: Vec<String> = file_contents.split("\n")
        .map(|s: &str| s.to_string())
        .collect();
    lines
}

Some things that seem suboptimal to me:

  • Two separate error checks for reading the file.
  • Reading the entire file to a String, which will be thrown away. This would be particularly wasteful if I only wanted the first N lines.
  • Making a &str per line, which will be thrown away, instead of somehow going straight from the file to a String per line.

How can this be improved?

2
  • 3
    Use the lines() iterator: doc.rust-lang.org/std/io/trait.BufRead.html#method.lines Commented Jun 12, 2015 at 11:02
  • 1
    I don't understand why this question has been downvoted. If it's considered too subjective, I propose that the idiomatic tag be removed, since it exactly describes the kind of thing I'm asking about. Commented Jun 12, 2015 at 15:13

2 Answers 2

40

DK.'s answer is quite right and has great explanation. However, you stated:

Read a file and get an array of strings

Rust arrays have a fixed length, known at compile time, so I assume you really mean "vector". I would write it like this:

use std::{
    fs::File,
    io::{prelude::*, BufReader},
    path::Path,
};

fn lines_from_file(filename: impl AsRef<Path>) -> Vec<String> {
    let file = File::open(filename).expect("no such file");
    let buf = BufReader::new(file);
    buf.lines()
        .map(|l| l.expect("Could not parse line"))
        .collect()
}

// ---

fn main() {
    let lines = lines_from_file("/etc/hosts");
    for line in lines {
        println!("{:?}", line);
    }
}
  1. As in the other answer, it's worth it to use a generic type that implements AsRef for the filename.
  2. Result::expect shortens the panic on Err.
  3. BufRead::lines handles multiple types of newlines, not just "\n".
  4. BufRead::lines also gives you separately allocated Strings, instead of one big glob.
  5. There's no reason to collect to a temporary variable just to return it. There's especially no reason to repeat the type (Vec<String>).

If you wanted to return a Result on failure, you can squash the implementation down to one line if you want:

use std::{
    fs::File,
    io::{self, BufRead, BufReader},
    path::Path,
};

fn lines_from_file(filename: impl AsRef<Path>) -> io::Result<Vec<String>> {
    BufReader::new(File::open(filename)?).lines().collect()
}

// ---

fn main() {
    let lines = lines_from_file("/etc/hosts").expect("Could not load lines");
    for line in lines {
        println!("{:?}", line);
    }
}
Sign up to request clarification or add additional context in comments.

2 Comments

Your 'down to one line' function seems to return the entire text as 1 line for me, your first worked.
@Blankman I think the issue is that you aren't handling the returned Result from lines_from_file. Option and Result implement IntoIterator, so that might be tricking you.
22

As BurntSushi said, you could just use the lines() iterator. But, to address your question as-is:

  • You should probably read Error Handling in Rust; those unwrap()s should be turned into ?s, with the function's result becoming a Result<Vec<String>, E> for some reasonable E. Here, we reuse the io::Result type alias.

  • Use the lines() iterator. The other thing you can do is read the whole file into a String and return that; there's a lines() iterator for strings as well.

  • This one you can't do anything about: file_contents owns its contents, and you can't split them up into multiple, owned Strings. The only thing you can do is borrow the contents of each line, then convert that into a new String. That said, the way you've put this implies that you believe creating a &str is expensive; it isn't. It's literally just computing a pair of offsets and returning those. A &str slice is effectively equivalent to (*const u8, usize).

Here's a modified version which does basically the same thing:

use std::fs::File;
use std::io::{self, BufRead};
use std::path::Path;

fn lines_from_file<P>(filename: P) -> io::Result<io::Lines<io::BufReader<File>>>
where
    P: AsRef<Path>,
{
    let file = File::open(filename)?;
    Ok(io::BufReader::new(file).lines())
}

One other change I made: filename is now a generic P: AsRef<Path>, because that's what File::open wants, so it will accept more types without needing conversion.

3 Comments

Neat! But doesn't returning a Result imply that if the file is unreadable, it will be the caller's problem? Maybe there's no way to avoid that while also being lazy about reading the file?
@NathanLong Using unwrap or panic! means that if the file is unreadable, the entire thread explodes and the caller dies without warning. If the caller doesn't care about this, they can just call unwrap on the result and get the same explody behaviour. Or they can actually decide what to do with the error. Either way, it has no impact on reading the file: both will cause the function to stop executing in one fashion or another.
Can you add an example of invocation of lines_from_file?

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.