kestrel
kestrel copied to clipboard
More user friendly Configuration API
This is a WIP and does a few things and is best reviewed commit by commit
- Rename some of the different
Configuration.from
methods to be more specific to avoid the method overloading which makes compiler messages more convoluted. - Extract
Configuration
out into it's own class since it's getting complicated - Introduce a DSL/Builder which makes the compiler errors a little more managable but still slightly awkward.
Calling the builder looks like this:
Configuration.Builder
.create(PizzaAggregate.Companion::create)
.update(PizzaAggregate::update)
.created(PizzaAggregate.Companion::created)
.updated(PizzaAggregate::update)
.build()
and changes an error that looks like this:
Error:(28, 27) Kotlin: Type inference failed: Cannot infer type parameter UpdateEvt in inline fun <reified CreationCmd : CreationCommand, CreationEvt : CreationEvent, CmdErr : CommandError, reified UpdateCmd : UpdateCommand, UpdateEvt : UpdateEvent, reified Agg : Aggregate> from(noinline create: (CreationCmd) -> Either<CmdErr, CreationEvt>, noinline update: Agg.(UpdateCmd) -> Either<CmdErr, List<UpdateEvt>>, noinline created: (CreationEvt) -> Agg, noinline updated: Agg.(UpdateEvt) -> Agg = ..., noinline aggregateType: Agg.() -> String = ...): Configuration<CreationCmd, CreationEvt, CmdErr, UpdateCmd, UpdateEvt, Agg>
None of the following substitutions
((PizzaCreationCommand) -> Either<PizzaError, PizzaCreated>,Aggregate.(PizzaUpdateCommand) -> Either<PizzaError, List<PizzaUpdateEvent>>,(PizzaCreated) -> Aggregate,Aggregate.(PizzaUpdateEvent) -> Aggregate,Aggregate.() -> String)
((PizzaCreationCommand) -> Either<PizzaError, PizzaCreated>,Aggregate.(PizzaUpdateCommand) -> Either<PizzaError, List<UpdateEvent>>,(PizzaCreated) -> Aggregate,Aggregate.(UpdateEvent) -> Aggregate,Aggregate.() -> String)
((PizzaCreationCommand) -> Either<PizzaError, PizzaCreated>,Aggregate.(PizzaUpdateCommand) -> Either<PizzaError, List<@ParameterName PizzaUpdateCommand>>,(PizzaCreated) -> Aggregate,Aggregate.(PizzaUpdateCommand) -> Aggregate,Aggregate.() -> String)
can be applied to
(KFunction1<@ParameterName PizzaCreationCommand, Either<PizzaError, PizzaCreated>>,KFunction2<PizzaAggregate, @ParameterName PizzaUpdateCommand, Either<PizzaError, List<PizzaUpdateEvent>>>,KFunction1<@ParameterName PizzaCreationEvent, PizzaAggregate>,KFunction2<PizzaAggregate, @ParameterName PizzaUpdateCommand, Either<PizzaError, List<PizzaUpdateEvent>>>)
into
Error:(26, 30) Kotlin: Type mismatch: inferred type is KFunction2<PizzaAggregate, @ParameterName PizzaUpdateCommand, Either<PizzaError, List<PizzaUpdateEvent>>> but PizzaAggregate.(PizzaUpdateEvent) -> PizzaAggregate was expected
I'm wondering whether we should abandon the configuration building options, and just insist on using TypedAggregate
etc.. The 'configuration args' approach that you're improving here is probably more friendly for ruby devs, as you note, but this is not Ruby and as you've discovered, setting this up to work and produce sensible compiler errors is very challenging. But, if you just set up the generic type params once-off in your aggregate class implementation, everything Just Works, and then the configuration is just a one-liner, and the compiler errors are already sensible. This is also in line with being an opinionated framework, which I think you're in favour of @williamboxhall. I think that part of being opinionated is making sure there is one, and only one (nice clean simple) way to do things
(aside: coming from a Python background here – I get very frustrated by things such as method aliases in Ruby! Now we have to choose one of several functionally identical ways to do a standard thing, and it may not look like what other devs chose in the same codebase. And now we have to run Rubocop to tell us to use one over the other!).
In Kotlin, i think the TypedAggregate
approach is closer to being the one way we should pick, if we have to pick one: it is fairly idiomatic Kotlin code, it is simple for Kotlin devs to understand and implement their approach. The difference in setup from what we do in Ruby is unfortunate, but we should lean in to Kotlin IMO instead of hacking it to be a bit more Ruby-ish.
The other way to be opinionated would be avoid the TypedAggregate
approach entirely and lean into the approach you've made improvements on here. But, you've had to jump through some hoops on the implementation side to get us there (and we have to remember/infer from type hints which order to call .create(d)/.update(d)
on the builder).
Another aside: I spent a bit of time this morning trying to write a nice fleshed out DSL (just to see if I could) based on your work here, so you could do something like:
configuration {
create(PizzaAggregate.Companion::create)
update(PizzaAggregate::update)
created(PizzaAggregate.Companion::created)
updated(PizzaAggregate::updated)
},
but I couldn't get the type inference to work unfortunately, so it ended up being just as verbose as the TypedAggregate
option
Ah thanks for the look @admackin! This has definitely gotten stale so I reckon hold off on a detailed review for now and I might rebase this and go over it again when I have a chance.