gouline / kapsule

Minimalist Kotlin dependency injection
MIT License
164 stars 7 forks source link

Limitation in transitive dependencies or bug? #9

Closed lukaspili closed 6 years ago

lukaspili commented 6 years ago

Hello, It seems I found a bug or a limitation on transitive dependencies between 2 modules:


class Deps(
        a: DependenciesA,
        b: DependenciesB
) : DependenciesA by a, DependenciesB by b, HasModules {
    override val modules = setOf(a, b)
}

interface DependenciesA {
    val foo: String
    val foobar: String
}

class ModuleA : DependenciesA, Injects<Deps> {
    override val foo: String = "foo"
    override val foobar: String by required { foo + bar }
}

interface DependenciesB {
    val bar: String
}

class ModuleB : DependenciesB, Injects<Deps> {
    override val bar: String by required { if (foo == "foo") "bar" else "i dont know" }
}

val deps = Deps(ModuleA(), ModuleB()).transitive()

The dependency flow is: a.foobar -> b.bar -> a.foo Result: When injecting Deps.a.foobar, it throws a NPE because b.bar is null.

I didn't dig into Kapsule, but I think it makes sense because when ModuleA is built, all dependencies are resolved directly, with a dependency on ModuleB which has itself a dependency on ModuleA.

An easy fix would be to split dependencies into 3 modules, in order to not have a "cyclic dependency on Module level". However it would be nice to support this, because this is fine on dependency level.

What do you think? Thanks.

gouline commented 6 years ago

The case you're describing isn't supported and, to be honest, with the current intentionally simple design, it's unlikely to ever be supported. This is probably the line of complexity, beyond which you'd be better off going with fully-featured dependency frameworks (e.g. Dagger).

lukaspili commented 6 years ago

Fair enough. I agree that Kapsule must be kept simple, and this is why I prefer it to any other DI solution on mobile. I would suggest to specify this kind of limitation in the documentation. When migrating from a fully-featured DI framework, this case is pretty common.

gouline commented 6 years ago

Good point, will add that to the documentation.