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

Support composite pattern via decorators/proxies #776

Closed
ashmind opened this issue Jul 22, 2016 · 4 comments
Closed

Support composite pattern via decorators/proxies #776

ashmind opened this issue Jul 22, 2016 · 4 comments

Comments

@ashmind
Copy link

ashmind commented Jul 22, 2016

I've started building a RegisterComposite extension (same as #297), and I run into a few things that could be done better if I had more access to Autofac logic.

For example, it would be useful to be able to reuse logic behind CollectionRegistrationSource instead of doing this manually.

My suggestion: either make CollectionRegistrationSource public and support a filter delegate or a virtual method; or expose IsGenericListOrCollectionInterfaceType and similar through a public helper.

@tillig
Copy link
Member

tillig commented Jul 22, 2016

We'll have to think about this. Not sure if it'll make it in soon or not. The problem with opening internals up is that, basically, we're then stuck with them public as part of the supported API forever. Even if we stop using them and go a different route, we can't just refactor and remove anymore.

I'll leave the issue open, but for now your best bet is to go the copy/paste route.

@alexmg
Copy link
Member

alexmg commented Jul 25, 2016

I'm interested in improving our decorator support and catering for the composite pattern aligns with that. I would be interested to see what your implementation would look like if you could make the changes you are proposing. Perhaps this is something you could do in a fork that I could reference?

@ashmind
Copy link
Author

ashmind commented Jul 25, 2016

@alexmg That's interesting -- I only considered superficial changes here to help reuse some code, but proper integration would look different.

In my current solution:

  1. You register multiple implementations of ISomething.
  2. You register a composite (I use RegisterComposite<> at the moment, but I'm now thinking it should be AsComposite<> instead).
  3. You can register more implementations after 2, but those need PreserveExistingDefaults.
  4. When you Resolve<ISomething> you get the composite (since it's the default), which uses a hacky Parameter override to avoid circular dependency.
  5. When you Resolve<ISomething[]> you get a mess (all implementations from 1 and 3, plus the composite).

In a perfect solution as I see it:

  1. You register multiple implementations of ISomething.
  2. You register a composite.
  3. You can register more implementations. Probably without needing PreserveExistingDefaults, since e.g. a plugin assembly wouldn't know there is a composite. But then it would need some other marker to override composite if it explicitly wants to.
  4. When you Resolve<ISomething> you get the composite (resolution should be done without weird parameter override — by having some hook between finding registrations and resolving them).
  5. When you Resolve<ISomething[]> you get an array with a single item — the composite.

I'll take a look at what's needed to change for a perfect solution, but I can't promise any specific timeline. Hopefully later this week.

@tillig tillig changed the title Please provide a way to reuse logic in CollectionRegistrationSource Support composite pattern via decorators/proxies Oct 27, 2017
@tillig
Copy link
Member

tillig commented Oct 27, 2017

Since the overall goal of the thing is to get composite support in, I've updated the title here.

We're pushing to enhance decorator syntax and features, which will hopefully resolve this issue. I've created a larger meta-issue for tracking that at #880. In the meantime, I'll close this specific issue and hopefully we can handle everything at once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants