Skip to content
This repository has been archived by the owner on Jul 3, 2022. It is now read-only.

Exception using injected property to initialize other properties #8

Closed
dalewking opened this issue Oct 22, 2017 · 8 comments
Closed
Assignees

Comments

@dalewking
Copy link

Here is a simple example with an injected property where I want to use the injected property to define another property:

class TestModule
{
    val bar = "bar"
}

class Test(module: TestModule): Injects<TestModule>
{
    init { inject(module) }
    val foo by required { bar }
    val foobar = "testing " + foo
}

fun main(args: Array<String>) { println(Test(TestModule()).foobar) }

Running this gives an exception:

Exception in thread "main" kotlin.KotlinNullPointerException
	at space.traversal.kapsule.Delegate$Required.getValue(Delegate.kt:44)
	at com.nobleworks_software.gmail_handler.Test.getFoo
	at com.nobleworks_software.gmail_handler.Test.<init>

i have verified that the call to inject happens before the foobar expression is evaluated and that the lambda passed to required is never invoked.

@dalewking
Copy link
Author

OK, discovered that it works if I move the init to after the property is defined since it appears that the injection gets the value and stores it in the delegate:

class Test(module: TestModule): Injects<TestModule>
{
    val foo by required { bar }
    init { inject(module) }
    val foobar = "testing " + foo
}

but that lead to another issue. What I really want in my real code is lazy evaluation. Say I have this for the module:

class TestModule
{
    val bar by lazy { someExpensiveCalculation() }
}

The way you are doing it this will call the expensive calculation as soon as the module is injected. What I want is that the property is only ever calculated if someone references the injected property foo.

@gouline
Copy link
Owner

gouline commented Nov 6, 2017

To ensure I understand your point correctly, do you mean that you want an additional option requiredLazy {} (and optionalLazy {}) for a lazy variant of the same injection? If so, I'll consider adding that in the next version.

@gouline gouline self-assigned this Nov 6, 2017
@dalewking
Copy link
Author

Those are probably necessary but not sufficient. Those require the injection site to specify whether the value is created at time of injection of at first use of injected property. The module should be able to control that as well.

What you really should do is go through the documentation for Kodein and all the different types of injection and see if and how that is supported in kapsule.

@gouline
Copy link
Owner

gouline commented Nov 7, 2017

The module cannot and should not control the injection. It already supports lazy instantiation (using lazy {}), which would currently delay it until the first injection. To delay it even further, until the first use of that injection (if I understand you correctly), the proposed requiredLazy {} and optionalLazy {} should be sufficient, right?

I want to stress that Kapsule is not aimed at supporting all use cases that Kodein or any other framework supports. The intention is to keep the injection simple and through direct references (so that you can click through to the provider from the injection site). Those who require more advanced functionality, should use the fully-featured libraries (e.g. Dagger), because they do it better.

@dalewking
Copy link
Author

The module cannot and should not control the injection.

The module controlling the injection is the whole point of dependency injection.

the proposed requiredLazy {} and optionalLazy {} should be sufficient, right?

Not in my mind. They are necessary in that you should also let the injected object also delay injecting the value.

I want to stress that Kapsule is not aimed at supporting all use cases that Kodein or any other framework supports.

So you actually want to create an inferior library?

The intention is to keep the injection simple and through direct references (so that you can click through to the provider from the injection site). Those who require more advanced functionality, should use the fully-featured libraries (e.g. Dagger), because they do it better.

And the direct references are the thing that I like about kapsule because it lets me avoid the extensive use of tags that you get with Kodein or Dagger, but the inability to control the instantiation as well as Kodein is a huge minus.

@gouline
Copy link
Owner

gouline commented Nov 26, 2017

The module controlling the injection is the whole point of dependency injection.

Incorrect. Modules are not the only part of the dependency injection flow, they are only there to provide the dependency, not control how it's injected/used.

So you actually want to create an inferior library?

I'm creating a library based on certain priorities, which I explained above. If some features are not possible without compromising these priorities, then yes, I'm perfectly happy to leave them out.

Not in my mind. They are necessary in that you should also let the injected object also delay injecting the value.

In that case, I'll welcome a pull request from you with a solution to this problem.

@dalewking
Copy link
Author

Incorrect. Modules are not the only part of the dependency injection flow, they are only there to provide the dependency, not control how it's injected/used.

Once again the point of DI is that the thing that is being injected should not be the one controlling the values that get injected. Something external to the thing being injected should be the one to control when, how, and what values are injected including when those values come into being.

In Kapsule that external thing is the module so the module should be the one that controls the values that get injected. It's OK to give the thing being injected some say which is why I say requiredLazy {} and optionalLazy {} are OK since they give the thing being injected some say, but the "module" should have the ultimate say.

The problem is that Kapsule has very limited control of when and how values are provided to the thing being injected. The more control you can give to the DI about how and when the values are provided the better.

In that case, I'll welcome a pull request from you with a solution to this problem.

I'll more likely just fork it and do the right thing.

@gouline
Copy link
Owner

gouline commented Nov 26, 2017

Up to you. My point is, if you manage to achieve "the right thing" without breaking direct references, I'm not against merging it into the main repository, in case others have a similar use case to yours.

Until then, I'll close this issue and you can reference it should you ever raise that pull request.

@gouline gouline closed this as completed Nov 26, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants