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

Parameters in by inject() are not lazy #67

Closed
Bodo1981 opened this issue Mar 2, 2018 · 6 comments
Closed

Parameters in by inject() are not lazy #67

Bodo1981 opened this issue Mar 2, 2018 · 6 comments
Milestone

Comments

@Bodo1981
Copy link

Bodo1981 commented Mar 2, 2018

After playing around a little bit with the new parameters i came accross another issue.

class MyFragment : Fragment() {
   val presenter: MyPresenter by inject(parameters = mapOf("activity" to activity!!)
   // or since support lib 27.1.0 we can call
   val presenter: MyPresenter by inject(parameters = mapOf("activity" to requiredActivity())
}

if you instantiate this fragment the presenter gets created instantly and not on his first call.
this leads to the following exception

Caused by: java.lang.IllegalStateException: Fragment MyFragment{3833b34} not attached to an activity.

currently a workaround is to inject the presenter like this

class MyFragment : Fragment() {
   val presenter: MyPresenter by lazy { (StandAloneContext.koinContext as KoinContext).get<MyPresenter>("",  mapOf("activity" to activity!!)) }
}

this works and the presenter gets only created after the first call

i think it would be great if by inject() has the same functionality as by lazy or if you will not change this direct instantiation you could provide another by injectLazy() function or so...

I am looking forward for your feedback

@arnaudgiuliani
Copy link
Member

Hello,

Koin is already heavily based on Lazy<T>{} expression on all its delegate properties.

Keep in mind that the dynmaic parameters that you pass, have to be laready evaluated.

Here below, a debug log from MVP example with Fragment injecting Presenter (https://github.com/Ekito/koin-samples/tree/master/samples/android-weatherapp):

Breakpoint reached at org.koin.sampleapp.view.result.ResultListFragment.onCreateView(ResultListFragment.kt:25)
Breakpoint reached at org.koin.sampleapp.view.result.ResultListFragment.onViewCreated(ResultListFragment.kt:31)
I/KOIN: Resolve class[org.koin.sampleapp.view.result.ResultListContract.Presenter] with Bean[class=org.koin.sampleapp.view.result.ResultListContract.Presenter, scope:WEATHER_LIST]
I/KOIN: 	Resolve class[org.koin.sampleapp.repository.WeatherRepository] with Bean[class=org.koin.sampleapp.repository.WeatherRepository]
I/KOIN: 	Resolve class[org.koin.sampleapp.util.rx.SchedulerProvider] with Bean[class=org.koin.sampleapp.util.rx.SchedulerProvider]
I/KOIN: 	Resolve class[org.koin.sampleapp.view.result.ResultContract.Presenter] with Bean[class=org.koin.sampleapp.view.result.ResultContract.Presenter, scope:WEATHER_LIST]
I/KOIN: 	(*) Created
I/KOIN: (*) Created

Presenter is injected for my first use of it. Check if you don't use one of the variable somewhere.

@gpulido
Copy link

gpulido commented Mar 5, 2018

Hello,
I'm having the same problem injecting viewModels.
I'm using simple-stack as a navigation / router, so I'm creating "basekeys" to navigate between fragments. This keys are not available in the fragment until the bundle is processed so I can't do this:

class ArcherDetailFragment:BaseFragment()
{
  val archerViewModel by
  viewModel<ArcherDetailViewModel>(parameters = mapOf(Properties.EXTRA_ARCHER_ID to (getKey<ArcherDetailKey>()?.id?:EmptyUUID)))
}

And I need to wait until the onActivityCreated is raised:

class ArcherDetailFragment:Fragment()
{
   lateinit   var archerViewModel:ArcherDetailViewModel
   override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)

        archerViewModel = getViewModel(parameters = mapOf(Properties.EXTRA_ARCHER_ID to (getKey<ArcherDetailKey>()?.id?:EmptyUUID)))
   }
}

The "getKey()" is a method of the baseFragment()

fun <T : BaseKey> getKey(): T? = arguments?.getParcelable("KEY")

I can live with the second option, but it looks less polished than the first :)

Regards
Gabriel

@Bodo1981
Copy link
Author

Bodo1981 commented Mar 5, 2018

I have found the issue!

The inject function is currently defined like this:

inline fun <reified T> ComponentCallbacks.inject(name: String = "", parameters: ParameterMap = emptyMap()) = lazy { (StandAloneContext.koinContext as KoinContext).get<T>(name, parameters) }

The problem here is, that we pass the ParameterMap directly to the lazy {} function. This ParameterMap gets created DIRECTLY when the fragment is created.

class MyFragment : Fragment() {
   val presenter: MyPresenter by inject(parameters = mapOf("activity" to activity!!)
}

At this moment the Activity is null, because all properties gets created when a new instance of the fragment is created.

The creation of the presenter is lazy like @arnaudgiuliani said, but the ParameterMap does not contain the activity as parameter.

A good solution for fixing this issue is to replace the ParameterMap with an paramsFunc() like below:

inline fun <reified T> ComponentCallbacks.inject(name: String = "", crossinline paramsFunc:() -> ParameterMap = { emptyMap() }) = lazy { (StandAloneContext.koinContext as KoinContext).get<T>(name, paramsFunc()) }

With this implementation the ParameterMap gets created at the same time when the presenter is first needed. At this moment the activity is already created and ready to use.

Maybe there are other possible solution to also provide the passed parameters at first creation!?

@arnaudgiuliani
Copy link
Member

Yes, interesting solution @Bodo1981 👍

@arnaudgiuliani arnaudgiuliani added this to the 0.9.1 milestone Mar 5, 2018
@fredy-mederos
Copy link
Contributor

The same with viewModel()...
I try with @Bodo1981 solution and it works

inline fun <reified T : ViewModel> LifecycleOwner.viewModel(
    fromActivity: Boolean = true,
    key: String? = null,
    name: String? = null,
    noinline paramsFunc: () -> ParameterMap = { emptyMap() }
): Lazy<T> {
    return viewModelByClass(fromActivity, T::class, key, name, paramsFunc)
}

fun <T : ViewModel> LifecycleOwner.viewModelByClass(fromActivity: Boolean = true, clazz: KClass<T>, key: String? = null, name: String? = null, paramsFunc: () -> ParameterMap = { emptyMap() }): Lazy<T> {
    return lazy { getViewModelByClass(fromActivity, clazz, key, name, paramsFunc()) }
}

@arnaudgiuliani
Copy link
Member

arnaudgiuliani commented Mar 5, 2018

Ok. Fixed.

I migrate all params to be functional. Koin has now a very nice syntax to supply dynamic params (lambda directly opened for parameters):

val presenter: MyPresenter by inject { mapOf("key" to myKey) }

Just migrate my MVP sample: https://github.com/Ekito/koin-samples/blob/0.9.1/samples/android-weatherapp/app/src/main/kotlin/org/koin/sampleapp/view/search/SearchActivity.kt#L23

You can play with Koin 0.9.1-alpha-5 👍

@arnaudgiuliani arnaudgiuliani changed the title by inject() is not lazy Parameters in by inject() are not lazy Mar 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants