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

Adds lazy loading support to binding methods #44

Merged
merged 2 commits into from
Aug 25, 2022

Conversation

wirecat
Copy link
Contributor

@wirecat wirecat commented Aug 16, 2022

Inspired from work by @Place1. Adds new Lazy methods that can optionally
be called to lazy bind a resolver to an abstract. The resolver will not be
called until the first time a resolve call is made.

A strong effort was made to not cause regressions or otherwise change the
behavior of the existing API. Minor changes were made to how validation
occurs and in some situations a more verbose error can be presented earlier
in a binding process.

Inspired from work by @Place1. Adds new Lazy methods that can optionally
be called to lazy bind a resolver to an abstract. The resolver will not be
called until the first time a resolve call is made.

A strong effort was made to not cause regressions or otherwise change the
behavior of the existing API. Minor changes were made to how validation
occurs and in some situations a more verbose error can be presented earlier
in a binding process.
@miladrahimi
Copy link
Member

miladrahimi commented Aug 17, 2022

Could you please explain why you need this feature?
If there is any problem with binding, I'll find it easily when I run my app, and I'll fix it before users face it.
I'm sure my HTTP handlers always run business logic not resolving dependencies.

BTW, the pipeline failed because of decreased test coverage.

@wirecat
Copy link
Contributor Author

wirecat commented Aug 17, 2022

@miladrahimi There's a tradeoff in doing this for sure and I see value in both lazy and non-lazy binding. It's why I felt it was critical to preserve both and not cause regressions in the non-lazy bindings.

In our use case we have multiple startup profiles and they have wildly different dependencies. It would be ideal if we could bind most of our dependencies in one place lazily and then only resolve them if a startup profile needs them. Currently we either need conditionals and/or to split bind calls across files to achieve this.

For our use case we can tolerate runtime faults, especially if it simplifies development/maintenance of our project.

@wirecat
Copy link
Contributor Author

wirecat commented Aug 17, 2022

I've updated my branch and believe there is 100% line coverage now.

@wirecat
Copy link
Contributor Author

wirecat commented Aug 24, 2022

@miladrahimi do you still have concerns about adding lazy functionality? Is this a direction you'd be comfortable with taking the project?

@miladrahimi
Copy link
Member

I've a question. What is the difference between transient and lazy transient? They both seem the same to me.

@wirecat
Copy link
Contributor Author

wirecat commented Aug 25, 2022

@miladrahimi I wasn't sure if I should implement lazy transient or not. It feels like low value add and could be removed in my opinion.

They're almost identical except for one change. Transient will call the resolver function when called, but lazy transient will not. Lazy transient defers calling the resolver until the first resolve.

@miladrahimi miladrahimi merged commit 18278ea into golobby:master Aug 25, 2022
@wirecat wirecat deleted the feature/add-lazy branch August 25, 2022 18:02
@miladrahimi
Copy link
Member

Thank you @wirecat. It's merged and release:
https://github.com/golobby/container/releases/tag/v3.3.0

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

Successfully merging this pull request may close these issues.

2 participants