Hi!
Couldn't resist an opportunity to nerd out about software architecture! I must say I really enjoy the overall theme here! However, I can't say I like the concrete implementation as much. There are two pragmatic, and two philosophical reasons for that.
The first pragmatic problem is that this is very complex code. Like the following line reads very much like trolling of the Rust language :)
conn: &mut <<<Self as BiddingStateStore>::Persistence as persistence::Persistence>::Connection as persistence::Connection>::Transaction<'a>,
The second pragmatic problem is that, because everything is generic, the build times, once this architecture is scaled up, are going to be atrocious -- there are no opportunities for separate compilation in this setup.
The first philosophical problem is that everything being generic doesn't make sense. The actual business rules should be agnostic of the implementation of the persistence, and not parametrized by it. It doesn't make sense that we, essentially, get two copies of the application -- one for in-memory stuff, one for postgress stuff.
The second philosophical problem is that there's little separation between the interface to plug the outside world into, and the interface we can rely on internally. Each extensibility point has two sides -- one for the provider of extension, and one for the consumer of the extension. The two interfaces often want to be a bit different, and it's nice to have a strict separation between the two. In the concrete terms, BiddingStateStore
has two methods which needs to be implemented, and two convenience methods for users to call. But it is possible to override convenience methods, which doesn't sit right with me.
Before proceeding with describing my own take on this, I want to quickly share a trick to reduce some syntactic repetition in the current impl. I think nested associated type lookups, like the one quoted, can be reduced by using type family pattern (not sure, haven't tried this refactor):
trait Persistance {
type Connection: Connection<Self>;
type Transaction: Transaction<Self>;
}
trait Connection<P: Persistance> {
fn start_transaction(&mut self) -> Result<P::Transaction>;
}
trait Transaction<P: Persistance> {
fn commit(self: Box<Self>) -> Result<()>;
}
The idea here is to have one main type, which binds together all other types. Then you parametrize stuff over P: Persistence
, and there's always one step to get any particular type.
Anyway, that's not how I'd do it. The solution I like arises from the two philosophical points.
First, we definitely want the stuff to be configurable, so we need to have some sort of trait Database
to allow for that. At the same time, we don't want the bulk of the code to care about potentially different databases at all. We really want the meat of the code to just deal with a database, so we want struct Database
. The way to code this might be like this:
pub struct Database {
imp: Box<dyn DatabaseImpl>
}
// This is the interface that most of our applications sees --
// just a concrete type with a bunch of methods
impl Database {
pub fn query(&self, query: &str) -> QueryResult {
self.imp.query(query)
}
pub fn query_foo(self) -> QueryResult {
self.query("foo")
}
}
// And this is the interface that `main` / `test` see --
// that's how you actually plug a custom store
pub trait DatabaseImpl {
fn query(&self, query: &str) -> QueryResult;
}
impl<T: DatabaseImpl> From<T> for Database {
fn from(imp: T) -> Database {
let imp = Box::new(imp);
Database { imp }
}
}
And what's the bulk of my PR is -- just ripping out type parameters and replacing them with concrete Persistance, Connection, Transaction types. The polymorphism is preserved though, because those concrete types store a polymorphic dyn
thing inside. In a sense, that's just an appliation of non-virtual interface pattern to Rust.
Pragmatically:
- this simplifies the code a lot -- we don't need to be generic everywhere
- this should improve compile times -- now most of the code is concrete, and can be compiled separatelly.
Curious what you think about it!