Problem Statement
I have a set of structs, A, B, C, and D, which all implement a trait
Runnable.
trait Runnable {
fn run(&mut self);
}
impl Runnable for A {...}
impl Runnable for B {...}
impl Runnable for C {...}
impl Runnable for D {...}
I also have a struct Config which serves as the specification for constructing A,
B, C, and D instances.
struct Config {
filename: String,
other_stuff: u8,
}
impl From<Config> for A {...}
impl From<Config> for B {...}
impl From<Config> for C {...}
impl From<Config> for D {...}
In my program, I would like to parse a Config instance and construct either an A,
B, C, or D depending on the value of the filename field, and then call
Runnable::run on it. The struct should be selected by sequentially checking each struct against a filename string and selecting the first one that "matches" that string.
Naïve Implementation
Here is a naïve implementation.
trait CheckFilename {
fn check_filename(filename: &str) -> bool;
}
impl CheckFilename for A {...}
impl CheckFilename for B {...}
impl CheckFilename for C {...}
impl CheckFilename for D {...}
fn main() {
let cfg: Config = get_config(); // Some abstract way of evaluating a Config at runtime.
let mut job: Box<dyn Runnable> = if A::check_filename(&cfg.filename) {
println!("Found matching filename for A");
Box::new(A::from(cfg))
} else if B::check_filename(&cfg.filename) {
println!("Found matching filename for B");
Box::new(B::from(cfg))
} else if C::check_filename(&cfg.filename) {
println!("Found matching filename for C");
Box::new(C::from(cfg))
} else if D::check_filename(&cfg.filename) {
println!("Found matching filename for D");
Box::new(D::from(cfg))
} else {
panic!("did not find matching pattern for filename {}", cfg.filename);
};
job.run();
}
This works but has some code smells:
- Giant
if else if else if else if else...statement is smelly IMO - Lots of repetition: The code used to check the filename, print which struct type was selected, and construct the instance from the config are the same for each branch and only differ in which struct type they're dealing with. Is there a way to abstract this repetition away?
- Very error prone: its very easy to accidentally screw up the mapping between filename
string and struct by failing to synchronize the struct with the predicate; for
example writing something like:
and the compiler would not catch this.if D::check_filename(&cfg.filename) { println!("Found matching filename for D"); Box::new(B::from(cfg)) // Developer error: constructs a B instead of a D. } - Adding new structs (e.g.
E,F,G, etc.) to the program is not very ergonomic. It requires adding a new branch for each to the mainif elsestatement. It'd be much nicer to simply add the struct to some sort of "master list" of srtuct types.
Is there a more elegant or idiomatic way of doing this that addresses these smells?
Since the conversion consumes
Config, the challenge in unifying the logic for all types is that you need to conditionally move the config value in order to convert it. The standard library has multiple cases of fallible consuming functions and the pattern they use is to returnResult, handing back the maybe-consumed value in theErrcase. For example,Arc::try_unwrapextracts the inner value of anArc, but if this fails it gives theArcback in theErrvariant.We can do the same here, creating a single function that produces one of the appropriate structs if the filename matches, but giving back the config on an error:
Then you can write another function with a static slice of specific instantiations of this function, and it can try each in order until one succeeds. Since we move the config into each loader function, we have to put it back in the
Errcase so the next loop iteration can move it again.This solves all of your concerns:
try_convert_config_toimplements the logic for all types one time.check_filenameandinto) on different types as long as you usetry_convert_config_to.CONFIG_LOADERSslice.(Playground)