@Instance(types) must accept a classifier per type

[sergejsha]

When declaring

Instance(
    types = [Foo::class, Bar::class],
    classifier = "foo"
)
internal class FooBar(): Foo, Bar {}

all types use shared classifier value. Expected behavior: classifier should be declarable per type.

A solution could be to remove types from the Instance annotation and apply a new InstanceAlias annotation for each new instance type as following:

@Instance(type = Foo::class, classifier = "foo")
@InstanceAlias(type = Bar::class, classifier = "bar")
internal class FooBar(): Foo, Bar {}

[realdadfish]

I'm not sold to the concept of having two types provided by one class like this. Why not do it like this (if Foo and Bar are interfaces, what they should be)?

@Instance(type = FooBar::class)
internal class FooBar(): Foo, Bar {}

@Instance(type = Foo::class, classifier = "foo")
fun provideFoo(foobar: FooBar): Foo = foobar

@Instance(type = Bar::class, classifier = "bar")
fun provideBar(foobar: FooBar): Bar = foobar

Yes, it is slightly more declarative work, but also more clean IMHO.

[sergejsha]

(if Foo and Bar are interfaces, what they should be)

This case is very specific, but sometimes there is no better way around. The typical one I came across was when I wanted to implement an extension interface (e.g. PlayerFeature) and the implementation of this interface wanted to hook up into another extensible thing thought another extension interface (e.g. PlayerListener). In this case Manget must ensure that exactly the same instance gets registered under two (or more) interfaces. Current version of Magnet offers the following syntax for it:

@Instance(types = [PlayerFeature::class, PlayerListener::class])
internal class UndoPlayerFeature(): PlayerFeature, PlayerListener {
    override fun onCustomAction(action: String, extras: Bundle?) // from PlayerFeature
    override fun onPlayerEvent(event: PlayerEvent) // from PlayerListener
}

Recently I needed it when I was implementing ViewModel delegates, where ViewModel itself was implemented using predictable state container pattern. There were two interfaces like NextButtonDelegate (or whatever other feature of the ViewModel) and Reducer<Change, State> one, which hooked up into ViewModel's state reduce function. "Out of the box" support for such cases is a neat feature, in my opinion.

Why not do it like this?

It's surely possible, but I would not do it like that because

1. Implementation class gets exposed to "outside" due to the first declaration, which is not always wanted.
2. Magnet cannot guarantee, that the second and the third declarations return the same instance as the the first one, which is essential.
3. Magnet will generate 3 factories instead of just one with aliases syntax.
4. It has busier syntax and is harder to read. This is a very subjective opinion though.

Proposed change does not add a new feature but suppose rather remove an artificial restriction of the current syntax.

[realdadfish]

Understood your points, I guess it is just a very specific use case that is modeled in a rather general purpose tool, but no hard feelings here. Just from an API point of view I'd say that the additional (mental) complexity that an aliasing mechanism brings onto the table would have to be reasoned by more than one particular, very specific use case.

Like, you couldn't (or maybe you could, but would you?) prevent misuse of the feature like this, right?

@Instance(type = Foo::class, classifier = "foo")
@InstanceAlias(type = Foo::class, classifier = "bar")
internal class Foo() {}

But again, no hard feelings :)

[sergejsha]

The more I use Magnet and apply interface segregation principle, the often I need this feature. I might be biased, but I see it as an advantage having this kind of syntax under my fingers then going chatty dagger way.

Like, you couldn't (or maybe you could, but would you?) prevent misuse of the feature like this, right?

My gut feeling tells this is not a misuse, but I cannot say anything certain about it. It needs to be tried out. If this syntax/concept causes issues, it can always be deprecated and replaced by a better one.

[sergejsha]

Postponed until there is a real need for it.