New feature added, giving a way an help to write your definitions without any get() function.
class View(val presenter : Presenter)
interface Presenter
class MyPresenter(val repository : Repository) : Presenter
class DataRepository()
class MyViewModel(val repository : Repository) : ViewModel()
For a gien module, written with get():
val myModule = module {
single { DataRepository() }
factory<Presenter> { MyPresenter(get()) }
single { View(get()) }
viewModel { MyViewModel(get()) }
}
we can simplify it like:
val myModule = module {
single<DataRepository>()
factory<Presenter> { create<MyPresenter>() }
single<View>()
viewModel<MyViewModel>()
}
The create() DSL function is available in Koin 1.0.0-beta-7 👍
How is this different to build<>() from the koin-reflect module?
Nice improvement!
What about params creation? Like
viewModel { params -> MyViewModel(params["args"], get()) }
The same thing as previous build() function, but generalized.
If you need to take hand on constructor (injection params for example), you still need to write your definition as a function.
This feature is at the moment in "experimental" state, and we need community feedback to help improve it.
Where for the others you can do it without the create syntax, i.e. viewModel<MyViewModel>(), it would be great to be able to do something similar for when you are creating an object of an interface, such as with the factory example. It would be great to be able to do something like factory<Presenter , MyPresenter>()
I like the brevity, but I suppose it relies on reflection?
Yeah, based on 2 lines of Java reflection to find your class constructor. No magic :) If you don"t want to write the function expression with the constructor, we have to find it for you.
I was thinking about the double type syntax... don't know if it's too "complex" to write?
To me the issue is you don't want all factories to be written as factory<Presenter , MyPresenter>(), so you have to have a different method name to cater for that and the single class case:
inline fun <reified T : Any, reified U : T> factoryInterface(
name: String = "",
override: Boolean = false,
noinline definition: Definition<T>? = null
): BeanDefinition<T> {
return if (definition == null) provide<T>(name, false, override, false) { create<U>() }
else provide(name, false, override, false, definition)
}
It's a shame reified types don't work on generics of generics otherwise to allow the passing of something like factory
The only other thought I had was whether you could do something with an infix function:
factory<MyPresenter>() asA Presenter::class
Actually I'm being a bit silly with the overloading, with the double type syntax the assumption would be you don't want a definition so the following overload would suffice:
inline fun <reified T : Any, reified U : T> factory(
name: String = "",
override: Boolean = false
): BeanDefinition<T> {
return provide<T>(name, false, override, false) { create<U>() }
}
This function doesn't seem to clash with the original function either, which I originally thought it might, so you can happily write all these together in a module:
factory<YetAnotherPresenter>()
factory<MyOtherPresenter> { create() }
factory<Presenter, MyPresenter>()
Of course this means any factories that require defining the constructor must use the single type function but I think that is just fine
Personally I'd like to move this feature to separate (seems it was removed in 1.0.0) package - koin-reflection, so developers has to opt-in to use reflection.
Why:
1.1. What is Koin?
Koin is pragmatic lightweight dependency injection framework for Kotlin developers. Written in pure Kotlin, using functional resolution only: no proxy, no code generation, no reflection. Koin is a DSL, a lightweight container and a pragmatic API.
create() function uses reflection and may use it accidentally in some Android code. Reflection on Android is costly operation and using it in Koin may slow down app startup time. Reflection should be opt-in option, not opt-out.Yep, that's why I've initially created the koin-reflect module and considered lastly to make it at least in a separate package extension (easier to create the viewModel() create version).
But, come back to a separate is a better way to protect users from using reflection accidentally 👍
I'm not so concerned personally about using reflection on Android, a lot of the tools we actively use rely on reflection already, whether that's retrofit (uses Proxy) for networking or GSON or Jackson for JSON processing. My understanding with Koin is its not going to instantiate all the definitions on startup, but only as required so the cost is spread out. Arguably your own hand-rolled bean definitions have to be careful about their cost of creating too
Granted I do think giving the option to opt in or out of reflection has its benefits. What are your thoughts on a startup parameter that defaults to reflection off and would throw an error if any definitions weren't provided?
If it is in a separate module is the proposed interface above for both single and dual types still possible as it would be a shame to lose that.
@mattmook it is totally ok to have reflection in Retrofit and GSON (or Jackson). The key point here is that this libraries are used on non-main Android thread and added execution time by reflection is much less compared to I/O.
On the other hand Koin will usually provide dependencies from Application start, and usually this dependencies are requested on the main thread.
Personally, I think better to limit developers possibilities to shoot in their own legs, so they should add/enable reflection manually, hopefully understanding the costs.
IMHO, separate module is the best solution, but I am also ok with off by default Koin configuration parameter.
I agree; reflection usage should be explicit opt in, either (my preference)
via a separate artifact or via configuration.
By the way, retrofit does not instantiate via reflection any of your
classes. You can't compare that to what Koin does here. And gson is again
special as it doesn't even require a constructor to be present...
On Mon, 27 Aug 2018, 19:01 Yahor Berdnikau, notifications@github.com
wrote:
@mattmook https://github.com/mattmook it is totally ok to have
reflection in Retrofit and GSON (or Jackson). The key point here is that
this libraries are used on non-main Android thread and added execution time
by reflection is much less compared to I/O.On the other hand Koin will usually provide dependencies from Application
start, and usually this dependencies are requested on the main thread.Personally, I think better to limit developers possibilities to shoot in
their own legs, so they should add/enable reflection manually, hopefully
understanding the costs.IMHO, separate module is the best solution, but I am also ok with off by
default Koin configuration parameter.—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/InsertKoinIO/koin/issues/196#issuecomment-416294395,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAL_ZUMKx2xGoELTmnNa-r-aatM-ZCW6ks5uVCXygaJpZM4WEDYv
.
One more point here: using java reflection directly is one step back regarding support for Kotlin JS(#120)/Native/Multiplatform.
Agreed, as a separate module does make it more of a thought step than being a toggle switch in the main module.
My point with Retrofit is that the retrofit.create(MyApi::class.java) method uses reflection, directly calling Proxy.newInstance which gets the constructor and calls newInstance i.e. it instantiates your class. What that means is I already have reflection in my dependency creation hierarchy. Reflection is considered slow but at times I think it gets a worse rep than it deserves. There was an interesting article from a while back that showed on Dalvik autoboxing to be far costlier than reflection Is Android reflection really slow. (obviously take any metric with a pinch of salt)
I would certainly be interested to see some performance metrics comparing Koin with and without reflection, and I do wonder if we should already be advocating initialising koin dependencies in background threads on Android already?
I was inspecting decompiled java code from Kotlin Bytecode and saw import java.lang.reflect.Constructor; on every definition. Does KOIN use reflection to get constructors?
with this feature yes.
I just tried non-beta styled get() only definition and it still uses reflection API.
Oh, I get it. I was trying with factory instead of provide.
I'll close this issue. I prefer to separate any reflection feature from the core. Keep in touch.