dig
dig copied to clipboard
uber-go
WIP provide params to invoke
Would it be an idea to let Invoke take arguments that aren't coming from the container?
It seems rather straight forward to add this, which leads me to belief it's not in there for a (good) reason...?
I understand this PR would break BC and it would be better to have a new method, but opening the pR atm purely for discussion as it's also missing any tests atm so it's not ready to be merged anyway.
If there's interest in merging it then I'll add InvokeWithParam or smt and add tests.
would be used something like;
// my super complicated service
type MyService struct {
Booyaa bool
}
// provide for my super complicated service
func NewMyService() *MyService {
return &MyService{}
}
func CanWeBooyaa(ctx *gin.Context, service *MyService) {
ctx.String(200, fmt.Sprintf("booyaa? %v! \n", service.Booyaa))
}
container := dig.New()
container.Provide(NewMyService)
ctx := &gin.Context{}
container.Invoke(CanWeBooyaa, ctx)
BAD FOR: Exposing to user-land code as a Service Locator.
I noticed this in the README, though I don't really understand why dig would be bad for this nor is there anything I can find in the go space that would be better for it.
Responded about the feature itself in #214.
BAD FOR: Exposing to user-land code as a Service Locator.I noticed this in the README, though I don't really understand why dig would be bad for this nor is there anything I can find in the go space that would be better for it.
The pattern we want to discourage is making the container available to the application and using it as a bag of things you can get on demand.
Something like,
func NewThing(c *dig.Container) *Thing {
var t Thing
c.Invoke(func(logger *Logger, metrics *Metrics) {
t.logger = logger
t.metrics = metrics
})
return &t
}
One of the major disadvantages of something like this is that your
dependencies aren't explicit. It's no better than using a
map[string]interface{} to pass parameters around. If I'm testing NewThing,
it's not clear exactly what objects are needed to test it. I have to read the
source code and build a container with the right objects in it.
Further, that's just a step removed from something like,
func NewHandler(c *dig.Container) *Handler {
return &Handler{c: c}
}
func (h *Handler) HandleRequest(...) {
h.c.Invoke(func(logger Logger) {
...
})
}
Which, in addition to having the same problems as the other case, is going to be significantly slower. We definitely don't want the container used on the request path.
you're saying significantly slower, but benchmarking dig shows its ~1500ns for a provide, that's not that bad ...
you're saying significantly slower, but benchmarking dig shows its ~1500ns for a provide, that's not that bad ...
Provide is not the only thing that needs to be benched, it's also the Invoke's. Dig is implemented using reflection and the general rule of thumb is "no reflection in the hot path".
