Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to directly call a constructor through dig #21

Closed
glibsm opened this issue Apr 21, 2017 · 7 comments
Closed

Add ability to directly call a constructor through dig #21

glibsm opened this issue Apr 21, 2017 · 7 comments

Comments

@glibsm
Copy link
Collaborator

glibsm commented Apr 21, 2017

Let's say that I have a constructor func (A, B) (C, error) { // codez }.

In order to partake in dig, I'd have to first Provide said constructor it to the graph, then declare a new variable of type C, and ask the graph to resolve:

container := dig.New()
container.Provide(func (A, B) (C, error) { // codez })
var c C
conatiner.Resolve(&c)
// c is now usable

I propose to add the Call function to Container interface, to make this process a lot smoother. Call would execute the function, inject the dependencies (without permanently adding it to the graph) and return the interface{} result (ready for casting).

The new experience would be:

container := dig.New()
c := container.Call(func (A, B) (C, error) { // codez }).(C)
// c is bueno to use

Not only this improves the API, but also allows users to create multiple objects of the same type that should they choose to.

@anuptalwalkar
Copy link
Contributor

Interesting. A few questions -

  1. How will this work when A and B are not provided? In order to Call the func, you need to make sure that A and B are provided and not creating a cycle.
  2. What happens for multiple return args? The way dig is structured, resolve c will get you the constructor execution without needing to initialize other variables, but then what if single return object is creating dependency cycle?
    e.g. c, _ := container.Call(func (A, B) (C, B, error) { // codez })

@glibsm
Copy link
Collaborator Author

glibsm commented Apr 24, 2017

  1. This case is easy - error return or a panic, since dig will fail to resolve A and B obviously.
  2. This is slightly more sophisticated, perhaps we only consider type, error returns for Call, or the api needs to be re-thunk. I.e. Call(func(), ...interface{}) the vararg of interfaces will be populated in the right order, or an error returned.

@anuptalwalkar
Copy link
Contributor

anuptalwalkar commented Apr 25, 2017

Ok, We can do this in 3 ways -

Option 1: return []interface{} to users which is equivalent to the return args in the function provided. Users can then go on and convert the objects to their given types.

	result := c.Call(func(o1 Object1, o2 Object2) (Ret1, Ret2, error) {
		return Ret1{}, Ret2{}, nil
	})
	c1 := result[0].(*Ret1)
	c2 := result[1].(*Ret2)

Option 2: variadic arguments for Call function allows objects to be resolved in the call itself.

	var c1 Ret1
	var c2 Ret2
	result := c.Call(func(o1 Object1, o2 Object2) (Ret1, Ret2, error) {
		return Ret1{}, Ret2{}, nil
	}, c1, c2)

Option 3: The Call function resolves and executes provided function, that assigns the resolved objects to the defined vars.

	var c1 Ret1
	var c2 Ret2
	c.Call(func(o1 Object1, o2 Object2) error {
		c1 = Ret1{}
		c2 = Ret2{}
		return nil
	})

@glibsm
Copy link
Collaborator Author

glibsm commented Apr 25, 2017

I'm really confused about Option 3. Lets say I wanted to use it with the and Call func (logger, config) *dispatcher to get access to the latter one.

How would my usecase look? I may not always be able to control the innards of the function to extern values from it.

@anuptalwalkar
Copy link
Contributor

I cleaned up a bit since there is no need of result. but something like this? -

	var d Dispatcher
	c.Call(func(cfg Config, l Logger) error {
		// Initialize dispathcher based on configuration and logging
		initialized_dispatcher := NewDispatcher(cfg, l)

		// assign initialized dispatcher to `d`
		d = initialized_dispatcher
		return nil
	})
	// use `d` constructed but not registered in dig.

@glibsm
Copy link
Collaborator Author

glibsm commented Apr 25, 2017

I see.

The really interesting thing for me for Option 3 is that we can completely remove .Resolve and .ResolveAll functions in favor of Call, or Invoke, or whatever else we end up calling it.

Consider the following setup:

c := dig.New()
// type1 -> type2 -> type3
c.Provide(func(*type1) *type2{})
c.Provide(func(*type2) *type3{})

In the current API, to get an instance of both type2 and type3 the following is required:

var t2 *type2
var t3 *type3
err := c.Resolve(&t3)
err := c.Resolve(&t2)
// or c.ResolveAll(&t2, &t3)

If we obsolete the two and rely more on the call/invoke functionality, we can determine which objects of type need to be extracted from the graph based on the function provided, i.e.:

err := c.Invoke(func(t2 *type2, t3 *type3) error {
  // t2 and t3 are ready to go!
  // log them, use them, store them, pass them around, etc
}

I think there is something interesting in here...

@anuptalwalkar
Copy link
Contributor

I am wary of using Invoke to replace Resolve because of locking. We need to remove locking for Invoke/Resolve if we want to use it extensively for callback.

hbdf pushed a commit to hbdf/dig that referenced this issue Jul 14, 2022
hbdf pushed a commit to hbdf/dig that referenced this issue Jul 14, 2022
* Add hook to register custom YARPC dispatcher

This will be used internally to override the YARPC dispatcher.

GFM-112
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants