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

Map multibindings does not support Lazy #1630

Open
cckroets opened this issue Oct 4, 2019 · 5 comments
Open

Map multibindings does not support Lazy #1630

cckroets opened this issue Oct 4, 2019 · 5 comments

Comments

@cckroets
Copy link

cckroets commented Oct 4, 2019

Currently Map multibindings can provide:
Map<Key, Value>
Map<Key, Provider<Value>>

I would like it to also provide:
Map<Key, Lazy<Value>>

Right now we use the Provider method. However our map only needs one instance of each Value. If we scope the Value we get one instance. If we leave Values un-scoped we get a new instance every time we access the map. Ideally each Value could be left un-scoped since they contain no state, but now we are creating many new instances of these objects unnecessarily.

@Chang-Eric
Copy link
Member

We'll need to think about the semantics of this. Right now for instance I think we'll always use the same map for Map<K, V> and Map<K, Provider<V>>. This would likely need to be different to give you a different map per injection, which may be confusing or complicate the code. Not that it is impossible to do but more just that it adds complexity for something that is likely not used that much. We'll have to consider the tradeoffs to see if this feature is worth it.

@bcorso
Copy link

bcorso commented Mar 12, 2021

If we scope the Value we get one instance. Ideally each Value could be left un-scoped since they contain no state, but now we are creating many new instances of these objects unnecessarily.

Just to throw it out there in case it helps someone, this is essentially what @Reusable was created for. It's a bit more work to scope each multibinding contribution, but it should be cheaper than Lazy since it doesn't require double-check-locking and it can cache across multiple classes.

@hadilq
Copy link

hadilq commented Mar 12, 2021

Thanks @bcorso for explanation! IMO, not providing Map<Key, Lazy<Value>> is just an inconsistency in the dagger API. My use case in #2474 was gathering some mapper classes in one parent mapper class to convert some types. I used their types as the key in the Map. In my case, it's efficient to initiate the mapper classes once per their parent, and keep it for future usage, also avoiding initiation of mappers that are not needed, which is the definition of Lazy. Clearly I cannot do it with @Reusable.

@bcorso
Copy link

bcorso commented Mar 12, 2021

IMO, not providing Map<Key, Lazy> is just an inconsistency in the dagger API.

Yeah, I think we mostly agree on that. However, this particular feature is not high priority at the moment so just wanted to mention some possible work arounds.

In my case, it's efficient to initiate the mapper classes once per their parent, and keep it for future usage, also avoiding initiation of mappers that are not needed, which is the definition of Lazy. Clearly I cannot do it with @Reusable.

@hadilq, just curious, is there a particular constraint that prevents you from using Map<Key, Provider<Value>> with @Reusable (or actual scope) on each Value? Sounds like it would get you similar behavior to what you're asking for.

@hadilq
Copy link

hadilq commented Mar 14, 2021

just curious, is there a particular constraint that prevents you from using Map<Key, Provider> with @reusable (or actual scope) on each Value? Sounds like it would get you similar behavior to what you're asking for.

@bcorso We are trying to reduce usage of @Reusabe in our project, because they act similar to singletons. Also the module, I was working on, has a dagger component that didn't provide scopes, so I tried to not touching everything to provide an efficient solution you suggested. But in the end, you are right, and we need to refactor our code to achieve that.

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

4 participants