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

Properly implement Constructor Injection for the DI pattern #629

Closed
robertmclaws opened this issue Jan 3, 2019 · 2 comments
Closed

Properly implement Constructor Injection for the DI pattern #629

robertmclaws opened this issue Jan 3, 2019 · 2 comments
Assignees
Labels
perf Potential performance improvements

Comments

@robertmclaws
Copy link
Collaborator

Currently, the product passes around IServiceContainers between classes and grabs whatever it needs whenever it needs it.

However, in order for the Restier architecture to be clean and thoroughly testable, it needs to properly implement Constructor-based Dependency Injection across the entire product surface. This will require a major refactor of the application, which is not likely to happen in the 1.0 timeframe. However, some things can be done in the interim to smooth the transition to that architecture.

@robertmclaws robertmclaws added the perf Potential performance improvements label Jan 3, 2019
@robertmclaws robertmclaws self-assigned this Jan 3, 2019
robertmclaws added a commit that referenced this issue Jan 3, 2019
- Reduce the number of allocations in the app by making the DefaultSubmitHandler instance-based and completing service lookups in the constructor rather than on every request.
An initial pass at tackling the architectural issues with #628.
Also starts to address the DI architectural concerns in #435 via #629, although those are a long way off from solving properly.
This was referenced Jul 17, 2019
@jspuij
Copy link
Contributor

jspuij commented Jul 19, 2019

Hi @robertmclaws. I've created a pull request for a first series of fixes that addresses this issue. It's PR #645. I've come across a lot of other issues that need a bit more discussion. I did not want to pollute the issues in this repository, so for now these issues are here:

https://github.com/jspuij/restier/issues

If you want me to move those issues over, I will. If you want to discuss them on something like Slack or Teams, i'll wait till that is set up.

The amount of classes that include a reference to Microsoft.Extensions.DependencyInjection have been reduced from 37 to 25. All classes that remain are either unit tests, or part of discussion about the design.

@robertmclaws
Copy link
Collaborator Author

I think we've gotten as far as we can in this release, so I'm going to go ahead and close this issue. If you think there is anything from your fix branch that needs to make it in before RTM, please let me know and we can talk about it.

Thanks!

robertmclaws added a commit that referenced this issue Dec 19, 2022
- Reduce the number of allocations in the app by making the DefaultSubmitHandler instance-based and completing service lookups in the constructor rather than on every request.
An initial pass at tackling the architectural issues with #628.
Also starts to address the DI architectural concerns in #435 via #629, although those are a long way off from solving properly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
perf Potential performance improvements
Projects
None yet
Development

No branches or pull requests

2 participants