proposition for churn loading

[ineiti]

not sure it is nicer - but this is what I tried to describe in the comment.

[tharvik]

The issue is that there is two errors and chaining theses mask one of them. We don't want to program to fail if the file doesn't exists (or do we?), simply to return an empty vector, but fail on an error from parse_churn.

If you want to write it as a chain of method call, you can do it as

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map(BufReader::new) // type: io::Result<BufReader>
    .map(parse_churn) // type: io::Result<anyhow::Result<Vec>>
    .unwrap_or(Ok(Vec::new())) // type: anyhow::Result<Vec> // erase File::open error
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

I'm not sure that the latter is more readable, your call.

[ineiti]

OK - thanks for explaining. What about this?

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map_or(BufReader::empty(), BufReader::new) // type: io::Result<BufReader>
    .map(parse_churn) // type: io::Result<anyhow::Result<Vec>>
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

or even

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map_or(Ok(vec![]), |f| parse_churn(BufReader::new(f))) // type: anyhow::Result<Vec>
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

[tharvik]

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map_or(BufReader::empty(), BufReader::new) // type: io::Result<BufReader>
    .map(parse_churn) // type: io::Result<anyhow::Result<Vec>>
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

BufReader::emtpy doesn't exists. I tried with io::empty but as the io::Read is a trait, it would need to be dyn so that both branches of the map_or have the same type.

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map_or(Ok(vec![]), |f| parse_churn(BufReader::new(f))) // type: anyhow::Result<Vec>
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

Indeed, it does work this way, well played! And that's more readable than my proposed version. You can even simplify it a bit by mapping the BufReader::new and I think that's the nicest we can do.

let churn = fs::File::open(churn_path) // type: io::Result<File>
    .map(BufReader::new) // type: io::Result<BufReader>
    .map_or(Ok(vec![]), parse_churn) // type: anyhow::Result<Vec> // erase File::open error
    .context("Failed to parse churn info.")?; // type: Vec // handle parse_churn error

[ineiti]

I misread the empty, it's indeed not in BufReader: https://doc.rust-lang.org/std/io/fn.empty.html

force-pushed your proposition - even though I prefer a single map_or over a map.map_or, but I consider it's your code ;)

[tharvik]

As this commit will get erased when it's merge with #74 I moved it to https://github.com/c4dt/arti/commit/e25510044ccc33cf3bb65b52147809807a17a8f4