Critique my factory pattern impl, please

[Bender-Rodriguez]

How's this look as a general pattern?

enum StrategyType {
    MeanReversion { strategy_params: Vec<String> },
}

trait Strategy {
    fn run(&self);
}

struct MeanReversion {
    strategy_params: Vec<String>,
}

impl MeanReversion {
    fn new(strategy_params: Vec<String>) -> Self {
        MeanReversion { strategy_params }
    }
}

impl Strategy for MeanReversion {
    fn run(&self) {
        todo! 
   }
}

struct StrategyFactory;
impl StrategyFactory {
    fn make(&self, typ: StrategyType, params: Vec<String>) -> Box<dyn Strategy> {
        match typ {
            StrategyType::MeanReversion { strategy_params } => {
                Box::new(MeanReversion::new(strategy_params))
            }
        }
    }
}

[paramagnetic]

You usually don't need the factory pattern in Rust. Getting a Box<dyn Trait> from a constructor is quite unusual and largely undesirable, precisely because it immediately erases the type and constrains the trait (e.g. it precludes having generic or Self-returning methods in the trait), both without the caller having a choice.

Rust's traits work over static types just fine, and you should generally work with those static types and expose generic functions, instead of making everything dynamic.

(To be honest, I find the StrategyFactory combination of patterns itself a suspiciously Java-esque red flag as well. You don't see this kind of code in most real-world Rust projects.)

[Bender-Rodriguez]

Thanks for your input.

There's no "StrategyFactory" here - I think maybe you're thinking of the GoF pattern "Abstract Factory." The "Strategy" in this code refers to a trading strategy.

When you say "constrains the trait," are you referring to trait objects? I do want these things to be trait objects, actually.

I am using trait objects for "large-scale" artifacts only (not pervasively) because I need to be able to switch implementations for unit testing. This requires dynamic dispatch - or so I thought.

Quoting from another thread here:

a given impl Trait cannot be created from several, distinct types. If you replace Box<dyn Trait> with impl Trait in the code above, it won't compile anymore. Or, to put it differently, impl Trait is not a proper type in itself, it's a placeholder for a concrete type which is known to the compiler, but not to the programmer. Therefore, you must treat it as a single type in a given context.

and

impl Trait is useful when you have a single, concrete type that you want to hide for some reason, or if you are simply too lazy to type it out (e.g. when it's a very complicated, nested Iterator adaptor).

I need dynamic dispatch to use dummy service implementations in unit tests. I don't think using static typing will work for this. Please correct me if I'm wrong.

[paramagnetic]

@Bender-Rodriguez

When you say "constrains the trait," are you referring to trait objects?

Yes. An object-safe trait can't have generic methods or methods that return Self, which is not something that you want by default.

@Bender-Rodriguez

I need to be able to switch implementations for unit testing. This requires dynamic dispatch

No, it doesn't. If your functions (that you are trying to test) are written properly and accept generic arguments bounded by the relevant trait, then they will – by definition – work with any type implementing that trait. Example.

[drewtato]

@Bender-Rodriguez

There's no "StrategyFactory" here

I'm not sure what you mean by this, but I agree with paramagnetic that it shouldn't be there. Instead, make should be a method on StrategyType.

impl StrategyType {
    fn make(self, params: Vec<String>) -> Box<dyn Strategy> {
        match self {
            Self::MeanReversion { strategy_params } => {
                Box::new(MeanReversion::new(strategy_params))
            }
        }
    }
}

What Rust code usually does instead of creating trait objects is put all the trait work inside the function. This is probably a big change, but should be possible. For this example, it would look something like this:

impl StrategyType {
    fn make_and_run(self, params: Vec<String>) {
        match self {
            Self::MeanReversion { strategy_params } => {
                MeanReversion::new(strategy_params).run();
            }
        }
    }
}

For more complex things, you may want to create another generic function to reduce duplication.

fn make_and_run(self, params: Vec<String>) {
    // used in all branches
    fn run(strategy: impl Strategy) {
        strategy.run();
    }

    match self {
        Self::MeanReversion { strategy_params } => {
            run(MeanReversion::new(strategy_params));
        }
    }
}

[Bender-Rodriguez]

@paramagnetic

No, it doesn't. If your functions (that you are trying to test) are written properly and accept generic arguments bounded by the relevant trait, then they will – by definition – work with any type implementing that trait. Example .

Yeah, not sure how I missed that. I think I got led astray by inaccurate info elsewhere.

For the record, I'm not a Java-OO type guy. I was many years ago, but the last ten have been mostly Scala, first a mix of OO and FP, and then (pure) FP all the way.

(I'm having a hard time now thinking of appropriate use-cases for dynamic dispatch other than callbacks and cases where implementors can't be known at compile-time...)

[Bender-Rodriguez]

But, to play devil's advocate a bit, this is an interesting read:

A failed experiment with Rust static dispatch - Julio Merino (jmmv.dev)

(I had forgotten about some of the negatives of static dispatch, like potential fn-copying code bloat..)

[drewtato]

@Bender-Rodriguez

(I'm having a hard time now thinking of appropriate use-cases for dynamic dispatch other than callbacks and cases where implementors can't be known at compile-time...)

The only case where it's truly required is for reducing binary size or compile times. For example, the format macros use trait objects to avoid creating specialized code for every invocation of println. And std::thread::spawn uses it since it's already essentially a dynamic function call. Going from static to dynamic calls can be several times slower, but going from one dynamic call to two dynamic calls is only twice as slow at most^[1].

Relatedly, generic functions in a library are always compiled by the user of a library, so if you have a lot of code that doesn't change, converting it to internally use trait objects can be a big win for incremental compilation. Sometimes you can even do this without trait objects, like with AsRef (example). The standard library is particularly concerned with this since it is compiled once by the Rust project for use in probably millions of rustc runs.

Other times it's useful for convenience, since generics may require inverting the flow of large parts of the code. It's also good for avoiding complex trait bounds like what async-trait does, since those are not technically impossible but would require making tons of custom Future types.

However, even if you use trait objects, I think it's better to make a proper generic function and instantiate it once with the trait object than to use trait objects throughout. Generics are far more versatile (you can add or remove Send + Sync + 'any_lifetime bounds at the instantiation site) and allow converting from trait objects to an actual type without having to edit every function (like when you want to wrap the trait object in a type).

@Bender-Rodriguez

But, to play devil's advocate a bit, this is an interesting read:

A failed experiment with Rust static dispatch - Julio Merino (jmmv.dev)

It sounds like using trait objects here is probably a good idea, but most of the stuff they show is just noise from adding trait bounds on structs. That's almost never necessary. They also should've combined traits way more than they did.

---

1. I think spawn is actually 3 dynamic calls, one to the OS, one to call the closure, and one to drop the closure. ↩︎

[jumpnbrownweasel]

The convention seems to be to only worry about code bloat due to monomorphization when it actually becomes a problem (or you have enough experience to know in advance it will be a problem), since using dynamic dispatch in Rust has added limitations. Here is a comparison from @quinedot.

[parasyte]

@drewtato

The only case where it's truly required is for reducing binary size or compile times.

Dynamic dispatch is truly required anywhere that needs type erasure. An example provided in The Book is storing a collection of mixed types. ^[1] I don't personally like using dynamic dispatch because it makes some things a lot harder, but there are certainly functional use cases for it beyond optimizations.

---

1. Enums are great for this, but they don't erase the owned type. It is not possible to store arbitrary types in an enum, but it is possible to store arbitrary types with dynamic dispatch. ↩︎

[drewtato]

But type erasure is never required. If the workaround is extensive, especially when cross-crate, then it should be used, but it's not required.

[parasyte]

It's required in the sense that a collection with static dispatch, like an enum, requires the author to think of every possible type they want the collection to own. With type erasure, the collection can store any type that implements the trait; including implementations that haven't even been written yet!

Concrete example: anymap. You don't need to submit a PR every time you want to add your own type to it.

[Bender-Rodriguez]

@drewtato

Other times it's useful for convenience, since generics may require inverting the flow of large parts of the code. It's also good for avoiding complex trait bounds like what async-trait does, since those are not technically impossible but would require making tons of custom Future types.

Can you please explain what you mean by "inverting the flow of large parts of the code"?

However, even if you use trait objects, I think it's better to make a proper generic function and instantiate it once with the trait object than to use trait objects throughout. Generics are far more versatile (you can add or remove Send + Sync + 'any_lifetime bounds at the instantiation site) and allow converting from trait objects to an actual type without having to edit every function (like when you want to wrap the trait object in a type).

Can you provide an example of this?

Thank you!

[jumpnbrownweasel]

Enums are great for this, but they don't erase the owned type. It is not possible to store arbitrary types in an enum, but it is possible to store arbitrary types with dynamic dispatch.

Is it only DSTs that cannot be stored (without boxing) in an enum?

(this is what I was trying to say when I accidentally edited your post)

---

Oh, based on your later post I think you meant that an enum is not open to additions by another crate, etc.

[parasyte]

Yeah, thanks for providing the clarification. I meant "arbitrary" in its purest form.

[drewtato]

@parasyte

Concrete example: anymap. You don't need to submit a PR every time you want to add your own type to it.

You can already use this statically because it's generic over the stored type. The downside is that the user needs to provide the enum instead of the library, and the library may need to export types with no public API so the user can include them in the enum, which is why it's usually used with dyn Any. Any is a bit special as far as traits go, so there's probably some things (particularly unsafe) that do require it, but not this one.

So if your requirement is to make a certain API, then trait objects might be necessary. But if you just need some functionality, then it's possible by other means.

[drewtato]

@Bender-Rodriguez

Can you please explain what you mean by "inverting the flow of large parts of the code"?

This is what I did in my reply where instead of returning Box<dyn Strategy>, I had the function do all the work inside. Depending on what you're doing, that could involve creating more traits or enums.

[paramagnetic]

Sometimes, it is. For example, how do you implement an HTTP router, where the handlers are functions of different concrete types, yet you have to handle them uniformly?

[drewtato]

A match, or you could do some hacky stuff with associated consts.

[paramagnetic]

You are missing the point. The routes are not compile-time constants. In a library, you'd have to make it general, so that a dynamic set of arbitrary, user-controlled routes can be mapped to arbitrary, user-controlled handlers. So essentially, what is needed is:

struct Router {
    ...
}

impl Router {
    fn route<F>(&mut self, path: &str, handler: F)
    where
        F: FnMut(Request) -> Response
    {
        ...
    }
}

How do you make this work so that it can be called with arbitrary paths and functions from the outside, if not with type erasure and a map from routes to dyn FnMut?