Read a file and get an array of strings

Question

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?

Answer 1

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