Runtime/Comptime configuration of an object

3 years ago, I made a reader that supported unpacking, and it was made via Box<dyn Read>. Yesterday, I revisited the code, and rewrote it, but what I got is very ugly.

I wanted the struct to receive an impl Read (for tests), and to implement Read itself -- this took a match clause with 3 identical one-liners, but that's ok.

What worries me is this with_pbar method. Unpack everything, try get file length, pack everything back.

I thought of just passing ProgressIter (which implements Read and wraps the reader), but it seems that if ProgressIter wraps the file, it slows things down. When it wraps BufReader instead, the whole code works 10% faster (the expensive part is parsing XML).

Is there a good example of configuring an object? (I can take both a runtime config, and compile time (compile time is like I have pbar in CLI, but not in tests/web server. Runtime is when a flag may change the object -- I guess, this requires dynamic Boxing.)

(Don't pay attention to Box<dyn Error>, I'll remove them later.)

pub enum FileUnpacker<R> {
	Gz(GzDecoder<BufReader<R>>),
	Bz(BzDecoder<BufReader<R>>),
	Plain(R)
}

impl<R: Read> FileUnpacker<R> {
	pub fn from_path(path: impl AsRef<Path>) -> Result<FileUnpacker<File>, Box<dyn Error>> {
		let f = File::open(&path)?;
		let fname = path.as_ref().file_name().ok_or_else(|| ReadError { msg: "file name is incorrect".into() })?.to_str().ok_or_else(|| ReadError { msg: "can't convert file name to strng".into() })?;
		Ok(if fname.ends_with(".gz") {
			FileUnpacker::Gz(GzDecoder::new(BufReader::new(f)))
		} else if fname.ends_with(".bz2") {
			FileUnpacker::Bz(BzDecoder::new(BufReader::new(f)))
		} else {
			FileUnpacker::Plain(f)
		})
	}
}

impl FileUnpacker<File> {
	fn file_len(fp: &File) -> Result<u64, Box<dyn Error>> {
		Ok(fp.metadata()?.len())
	}

	/// Unpacks files from the reader and packs them back with ProgressBar.
	pub fn with_pbar(self) -> Result<FileUnpacker<ProgressBarIter<BufReader<File>>>, Box<dyn Error>> {
		let style = indicatif::ProgressStyle::with_template("[{elapsed_precise}] [{wide_bar:.cyan/blue}] {bytes}/{total_bytes} ({eta})")?
			.with_key("eta", |state: &indicatif::ProgressState, w: &mut dyn std::fmt::Write| write!(w, "{:.1}s", state.eta().as_secs_f64()).unwrap())
			.progress_chars("#>-");
		let make_pbar = |len| indicatif::ProgressBar::new(len)
			.with_style(style.clone()).with_finish(indicatif::ProgressFinish::AndLeave);

		let make_br = |file, len| make_pbar(len).wrap_read(BufReader::new(file));
		Ok(match self {
			FileUnpacker::Bz(b) => {
				let mut file =  b.into_inner().into_inner();
                // these pieces get repeated, but I can't DRY it with another match statement                                      
                // because self is consumed here.
				let len = Self::file_len(&file)?;
				file.rewind()?;
				FileUnpacker::Bz(BzDecoder::new(BufReader::new(make_br(file, len))))
			},
			FileUnpacker::Gz(g) => {
				let mut file = g.into_inner().into_inner();
				file.rewind()?;
				let len = Self::file_len(&file)?;
				FileUnpacker::Gz(GzDecoder::new(BufReader::new(make_br(file, len))))
			},
			FileUnpacker::Plain(mut file) => {
				let len = Self::file_len(&file)?;
				file.rewind()?;
				FileUnpacker::Plain(make_br(file, len))
			}
		})		
	}
}

impl<R: Read> Read for FileUnpacker<R> {
	fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
		match self {
			FileUnpacker::Bz(r) => r.read(buf),
			FileUnpacker::Gz(r) => r.read(buf),
			FileUnpacker::Plain(r) => r.read(buf)
		}
	}
}

I think the main problem is, you mixed two responsibilities into one struct:

  • A builder: FileUnpacker::from_path(...).with_pbar()
  • A Read implementer.

I would suggest to simply provide two different constructors:

  • FileUnpacker::new(path: impl AsRef<Path>)
  • FileUnpacker::new_with_pbar(path: impl AsRef<Path>)

Then move the different construction steps into separate associated functions, and both new-constructors become two- or three-liners.

Off-topic, but would that three-armed match still work if you replaced it with FileUnpacker::Bz(r) | FileUnpacker::Gz(r) | FileUnpacker::Plain(r) => r.read(buf)?

No, the r binding in different cases would have different types. match is still strictly typed, it's not a duck-typed macro.