wire icon indicating copy to clipboard operation
wire copied to clipboard

Idea: Provide array with multiple provider

Open fzerorubigd opened this issue 4 years ago • 16 comments

Is your feature request related to a problem? Please describe.

I have several controllers that I want to create. These controllers all implement the type XX interface but are from different classes. Currently, I must create one function for each controller, and then call them one by one to create an array of controllers and then pass this array of controllers to the router package. The issue is all of them requires an instance of the same objects over and over. this object is shared between them (some validator, and also an object related to the database)

I have this problem in another place with loading multiple keys in the system from various sources. Usually, when I need multiple-instance or implementation of an interface.

Describe the solution you'd like

When the providers provide type X more than one time, and if the next consumer required a slice of type X (or it uses ...) then instead of error about multiple providers, pass them in order of appearance in the list, to the next function.

something like this

func base() string { return "shared resource" }
func p1(b string) int { return 20 }
func p2(b string) int { return 22 }
func p3(in ...int) int64  { 
    sum := 0  
    for i := range in {
        sum+= in[i]
    }
    return int64(sum)
}

func wireMe() int64 {
  panic(wire.Build(base, p1, p2, p3))
}

the result should be something like this :

func wireMe() int64{
   s := base() // this resource is shared between all of them
   return p3(p1(s), p2(s)) 
}

Describe alternatives you've considered

The only alternative is to write them one by one. The problem is the base function in my example. It should be some singleton in my use case, or I should call it myself and pass the resource to all other function as a parameter.

Additional context

None.

fzerorubigd avatar Aug 16 '19 19:08 fzerorubigd

Thanks for the suggestion!

wire generally does not work well with multiple providers of the same type: https://github.com/google/wire/blob/master/docs/faq.md#what-if-my-dependency-graph-has-two-dependencies-of-the-same-type

I suspect you're best off writing a single constructor that takes the concrete types as arguments (each concrete type implementing the interface, but use the concrete type not the interface), and use providers that also return each concrete type. wire will construct all of the concrete types for you, and call the constructor for you. If you add a new type, you'll need to add a provider for it and update the single constructor. Something like:

// A, B, C are structs that implement the Controller interface.
func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

// Router needs a slice of Controller.
type Router struct {
  Controllers []Controller
}

func makeRouter(a *A, b *B, c *C) *Router {
  return &Router{Controllers: []Controller{a, b, c}}
}

func wireMe() *Router {
  panic(wire.Build(controllerA, controllerB, controllerC, makeRouter))
}

Adding controllerD is straightforward -- new provider function, add to wire.Build, and add to makeRouter.

In practice, I expect you'll need to do something like this anyway, since the providers for each controller may take their own arguments that need to be created or passed in to wireMe.

vangent avatar Sep 10 '19 17:09 vangent

@fzerorubigd @vangent Vangent's example could be done with wire.Struct as well, right? I wonder if one way to implement a slightly more flexible feature in an idiomatic way would be a wire.Slice (or wire.Array) helper that functioned like wire.Struct but for an indeterminate number of things of the same type.

In my case, I am trying to write a "Dataset" library which could be backed by either xlsx or an sql table (for example), but with the presumption that consumers want to use particular tables. By design the constructor should take a number of columns, either as a variadic function, or via an explicit slice.

The above example for the controller/router would be expressed using this idiom as:

// A, B, C are structs that implement the Controller interface.
func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

func makeRouter(controllers ...Controller) *Router {
  return &Router{Controllers: controllers}
}

func wireMe() *Router {
  panic(wire.Build(
    wire.Slice([]Controller{}, controllerA, controllerB, controllerC), makeRouter)))
}

The point is that makeRouter doesn't have to change to add controllers. This could be done with wire.Struct perhaps, but only if makeRouter used reflect to build the array in its constructor, which is painful.

shaunc avatar Oct 28 '19 02:10 shaunc

@fzerorubigd @vangent @shaunc I submitted an implementation on #245, you can go and see, and give some implementation suggestions

zeromake avatar Apr 01 '20 06:04 zeromake

Hey @zeromake. I've replied on the PR. Let's discuss and agree upon the API first before hopping into code.

zombiezen avatar Apr 04 '20 16:04 zombiezen

@zombiezen So is the API design of wire.Slice inappropriate? Any suggestions?

zeromake avatar Apr 04 '20 23:04 zeromake

@zombiezen Using issues to communicate is somewhat inefficient, is there any other instant chat solution

zeromake avatar Apr 08 '20 08:04 zeromake

So is the API design of wire.Slice inappropriate? Any suggestions?

Echoing my guidance I left on the PR:

Consider the controller/router example in #207 for a large application. With the approach in this PR, one provider set at the end would have to know about all the particular controllers. A more scalable approach would be to have each controller provider set contribute its controller to the router's controller list binding, like with Dagger's multibindings.

If this is a feature you would like to contribute to Wire, we'd like to help you with a design discussion upfront, so we can give feedback before you get into coding.


Using issues to communicate is somewhat inefficient, is there any other instant chat solution

No. We've found asynchronous communication to work well for making design decisions across a distributed team and for having a record of how we arrived at our decisions. The maintainers of Wire (including myself) aren't working on this full-time, so our bandwidth is limited. Being able to leave feedback on a whole idea rather than a partial idea developed in real-time utilizes the time of both the reviewer and the proposer most efficiently. Even larger-staffed open source projects have similar policies.

zombiezen avatar Apr 08 '20 14:04 zombiezen

@zombiezen I do n’t understand what you mean, I actually followed the #207 plan

// A, B, C are structs that implement the Controller interface.
func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

func makeRouter(controllers ...Controller) *Router {
  return &Router{Controllers: controllers}
}

func wireMe() *Router {
  panic(wire.Build(
    wire.Slice([]Controller{}, controllerA, controllerB, controllerC), makeRouter)))
}

Extended support wire.Slice([]Controller{}, wire.Value(controllerA()), wire.Struct(controllerB(), "*"), wire.Set(controllerC))

zeromake avatar Apr 09 '20 02:04 zeromake

@zombiezen I would like to potentially take this on as I think this is a really useful feature that would remove even more boilerplate. Am I correct in interpreting the following:

A more scalable approach would be to have each controller provider set contribute its controller to the router's controller list binding, like with Dagger's multibindings.

As the following with the given example?

func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

func makeRouter(controllers ...Controller) *Router {
  return &Router{Controllers: controllers}
}

var controllerABSet = wire.Set(
  wire.Bind(new(Controller), new(*A)),
  wire.IntoSlice(controllerA),
  wire.Bind(new(Controller), new(*B)),
  wire.IntoSlice(controllerB),
)

func wireMe() *Router {
  panic(wire.Build(
    controllerABSet,
    wire.Bind(new(Controller), new(*C))
    wire.IntoSlice(controllerC),
    makeRouter,
  ))
}

codyleyhan avatar Dec 02 '20 19:12 codyleyhan

You're definitely on the right track, @codyleyhan. Hand-waving, I think wire.IntoSlice should take in a single provider-like thing. The problem is that those three wire.Binds in your example would conflict. This would result in this modified example:

func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

func makeRouter(controllers ...Controller) *Router {
  return &Router{Controllers: controllers}
}

var controllerABSet = wire.Set(
  controllerA,
  wire.IntoSlice(wire.Bind(new(Controller), new(*A))),
  controllerB,
  wire.IntoSlice(wire.Bind(new(Controller), new(*B))),
)

func wireMe() *Router {
  panic(wire.Build(
    controllerABSet,
    controllerC,
    wire.IntoSlice(wire.Bind(new(Controller), new(*C))),
    makeRouter,
  ))
}

There's a few other design considerations to keep in mind:

  • One of Wire's selling points is that unused providers are errors. As soon as you depend on a multi-binding, it brings in all of them. This might be okay for routes and other application level features, but this does permit for ambient addition of things into a provided value that's hard to trace. It gave me enough pause when prototyping to leave the feature out of the MVP. Not a non-starter, but something to consider.
  • The order of the resulting slice should be undefined and should probably be randomized, much like Go maps, to avoid code accidentally depending on order. This does make the generated code more complex, but is definitely worth it for the overall benefits. You can probably even use Go map iteration in the generated code to make the order undefined without introducing additional dependencies.

zombiezen avatar Dec 03 '20 00:12 zombiezen

@zombiezen That sounds reasonable to me.

Thinking about the following example more, it feels a bit unintuitive as to where the wire.IntoSlice should go when wire.Set is thrown in to the mix.

func base() string { return "shared resource" }
func p1(b string) int { return 20 }
func p2(b string) int { return 22 }
func p3(in ...int) int64  { 
    sum := 0  
    for i := range in {
        sum+= in[i]
    }
    return int64(sum)
}

var p1p2Set = wire.Set(
  p1,
  p2,
  // wire.IntoSlice(new(int)) can possibly be defined here
)

func wireMe() int64 {
  panic(wire.Build(
  base,
  p1p2Set,
  p3,
  wire.IntoSlice(new(int)) // can also be defined here
))
}

Maybe a better interface could be something like the following which removes the need to define wire.IntoSlice in a particular place and makes it a bit more clear that a particular provider will be contributing to the slice:

func base() string    { return "shared resource" }
func p1(b string) int { return 20 }
func p2(b string) int { return 22 }
func p3(in ...int) int64 {
	sum := 0
	for i := range in {
		sum += in[i]
	}
	return int64(sum)
}

// extension question
func p4() int64 {
  return 1
}

func p5(a int64) {} 

var p1p2Set = wire.Set(
	wire.IntoSlice(new(int), p1),
	wire.IntoSlice(new(int), p2),
)

func wireMe() int64 {
	panic(
		wire.Build(
			base,
			p1p2Set,
			wire.IntoSlice(new(int), p3),
                        // should the following still be allowed?  
                        p4,
                        p5, // arg would be provided from p4
		),
	)
}

One thought I also had that I added to the previous example is if we should allow for both []int and int arguments in the same set?

Please let me know your thoughts

codyleyhan avatar Dec 03 '20 02:12 codyleyhan

I would also really like this implemented. With some guidance, I could also work on it. It's an important feature that would add a lot of useability for us (and others).

@codyleyhan did you ever solve this, or find a workaround?

@zombiezen pinging, how do you look at this now?

Davincible avatar Oct 18 '22 23:10 Davincible

I am using the solution provided by @vangent. And it would be nice if we could simplify it by providing a provider function directly inside the build function. It would look like this:

// A, B, C are structs that implement the Controller interface.
func controllerA() *A {...}
func controllerB() *B {...}
func controllerC() *C {...}

// Router needs a slice of Controller.
type Router struct {
  Controllers []Controller
}

func makeRouter(c []Controlle) *Router {
  return &Router{Controllers: c}
}

func wireMe() *Router {
  panic(wire.Build(
    controllerA,
    controllerB,
    controllerC,
    func (a *A, b *B, c *C) *Router {
      return makeRouter([]Controller{a, b, c})
    }
))
}

jozolam avatar May 24 '23 21:05 jozolam

Hi everyone,

Any news on this issue ? I faced the same problem for my application and this could solve an ugly big constructor

FournyP avatar Oct 31 '23 21:10 FournyP

Ping. This feature would be neat indeed.

osousa avatar Feb 16 '24 14:02 osousa