Add support for IntoSet/Map(multiple = true) #467
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #249
Initial stab at implementing. Haven't added significant tests or updated any docs -- wanted to check this is the right direction first.
Using
@IntoSet(multiple = true)
requires returning aSet
. Using@IntoMap(multiple = true)
requires returning aMap
. In either case, all providers will be merged together the final collection. This also makes it possible to bind an emptySet or emptyMap to act as the default.A few notes:
IntoSet impl
This changes the
IntoSet
implementation fromsetOf()
to something like:buildSet(3) { add(itemOne()) add(itemTwo) addAll(itemThreeAndFour()) }
3
here is just the number of providers -- we can't know how many items amultiple = true
could provide, so our guess is just 1 item per provider. This is the same as what dagger generates for@ElementsIntoSet
, e.g.ImmutableSet.builderWithExpectedSize(3)...
.add
andaddAll
to properly distinguish between amultiple = true
value and amultiple = false
value (+=
could use the wrong overload for aSet<Iterable<T>>
).IntoMap impl
Similarly, this changes the
IntoMap
implementation frommapOf()
to something like:+=
operator since we know the values are eitherPair
orMap
-- it's unambiguous. Usingput
isn't directly possible -- there's noput(Pair)
.IntoSet w/ Lazy or Functions
The existing behavior is a little tricky. Let's say you have a
Set<Lazy<T>>
that you want to bind (same thing goes for() -> T
akaFunction0<T>
)lazy { }
.Lazy<T>
, that will cause all of the IntoSet providers of type T to be ignored. Only the lazy providers will be used, and no wrapping happens.This is a bit surprising, IMO. However, I'm not sure how this should interact with
multiple = true
. If you want a binding ofSet<Lazy<T>>
, but you have amultiple = true
provider ofSet<T>
, then the provider would have to be evaluated in order to find all the items, defeating the point of Lazy.Example:
Some options:
multiple = true
is not allowed for a binding ofSet<Lazy<T>>
provideSetT()
is not used in the generated code (butprovideT()
is).lazy { }
, something likeaddAll(provideSetT().map { lazy { it } })