init commit #42
init commit #42
Conversation
| super.onCreate() | ||
| appComponent = DaggerAppComponent.builder().appModule(AppModule(this)).build() | ||
| activityComponent = DaggerActivityComponent.factory().create(appComponent) | ||
| fragmentReceverComponent = DaggerFragmentReceiverComponent.factory().create(activityComponent) |
There was a problem hiding this comment.
Зачем тебе эти компоненты на уровне апликейшена? это чревато утечками памяти, тк в этих комопнентах лежит ссылка на фрагменты. Создавай их прямо в активити/фрагментах
| } | ||
|
|
||
| @Module | ||
| class FragmentReceiverModule(var context: Context) { |
There was a problem hiding this comment.
Не используй конструктор модуля, это bad practice тк ты не можешь использовать статические Provides методы и модуль теперь содержит стейт.
| @FragmentScope | ||
| @Component(dependencies = [ActivityComponent::class], modules = [FragmentReceiverModule::class]) | ||
| interface FragmentReceiverComponent { | ||
| var context: Context |
There was a problem hiding this comment.
Зачем эта проперти? Выглядит как какая-то смесь между сабкомпонентами и компонент депенденсис. Если ты выбрал сабкомпоненты то эти провижен методы/проперти не нужны
| @Component(dependencies = [ActivityComponent::class], modules = [FragmentReceiverModule::class]) | ||
| interface FragmentReceiverComponent { | ||
| var context: Context | ||
| var vmReceiver: ViewModelReceiver |
|
|
||
| @Component.Factory | ||
| interface Factory { | ||
| fun create(activityComponent: ActivityComponent): FragmentProducerComponent |
There was a problem hiding this comment.
А зачем ему в аргументах ActivityComponent если у тебя FragmentProducerComponent должен быть сабкомпонентом у ActivityComponent? в ActivityComponent должен появится провижен метод который отдает FragmentProducerComponent.Factory
|
|
||
| @Module | ||
| class ActivityModule { | ||
| @get:Provides |
There was a problem hiding this comment.
Этот модуль можно убрать. Добавь инжект аннотацию над ColorFlow
| @Module | ||
| class ActivityModule { | ||
| @get:Provides | ||
| @ActivityScope |
| interface ActivityComponent { | ||
| var context: Context | ||
|
|
||
| fun injectInto(producer: FragmentProducer) |
There was a problem hiding this comment.
Зачем эти методы? Если у тебя компонент на активити то писать мембер инжектор методы на фрагмент это бед практис. Если тебе во фрагменте нужна какая то сущность, которая предоставляется ActivityComponent и его модулями, то для этого как раз и используются сабкомпоненты app -> activity -> fragment
| private val context: Context | ||
| ) { | ||
| ) : ViewModel() { | ||
| @Inject |
There was a problem hiding this comment.
А почему здесь инжект в поле? Инжекти в конструктор и ВМ создавай через фабрику, это предпочтительный способ, к тому же я не совсем понял как у тебя вообще код компилируется если здесь нет мембер инжектора
| override fun onCreate(savedInstanceState: Bundle?) { | ||
| super.onCreate(savedInstanceState) | ||
| setContentView(R.layout.activity_main) | ||
| App.appComponent.injectInto(this) |
There was a problem hiding this comment.
Опять же, если у тебя компонент создается в Application и по смыслу и по названию относиться к Application, то его инжектить можно только в Application, если тебе нужны его сущности то выстраивай иерархию
|
|
||
| @FragmentScope | ||
| @Binds | ||
| fun bindColorGenerator(colorGeneratorImpl: ColorGeneratorImpl): ColorGenerator |
There was a problem hiding this comment.
Смотри, у тебя ColorGeneratorImpl класс, который стейта не содержит, тоесть разницы между двумя экземплярами никакой не будет, в таком случае лучше делать unscoped зависимость, таким образом ты сэкономишь на synchronized блоке
|
|
||
| @FragmentScope | ||
| @Binds | ||
| fun bindColorGenerator(colorGeneratorImpl: ColorGeneratorImpl): ColorGenerator |
There was a problem hiding this comment.
А почему у тебя ColorGenerator в двух модулях биндится? Если класс используется в 2 дочерних компонентах, мне кажется проще его в ActivityComponent положить?
| import javax.inject.Scope | ||
|
|
||
| @FragmentScope | ||
| @Component(dependencies = [AppComponent::class, ActivityComponent::class], modules = [FragmentReceiverModule::class]) |
There was a problem hiding this comment.
Я думаю тебе правильнее сделать ActivityComponent зависимым от AppComponent, а оба компонента фрагментов зависимыми от ActivityComponent. Тогда у тебя получается понятная древовидная структура, в реальном приложении обычно так и происходит
| @Module | ||
| interface FragmentReceiverModule { | ||
|
|
||
| @FragmentScope |
There was a problem hiding this comment.
То же самое про скоуп
| } | ||
|
|
||
| @Qualifier | ||
| annotation class AppScope No newline at end of file |
There was a problem hiding this comment.
Мне кажется неправильно Qualifier аннотацию называть постфиксом Scope, это запутать может
| class ViewModelReceiver( | ||
| private val context: Context | ||
| ) { | ||
| class ViewModelReceiver @Inject constructor( |
There was a problem hiding this comment.
Тебе здесь Inject аннотация не нужна, ты же через фабрики создаешь
|
|
||
| fun activityContext(): Context | ||
|
|
||
| val observer: MutableLiveData<Int> |
There was a problem hiding this comment.
Правильнее делать именно методы
|
|
||
| fun activityContext(): Context | ||
|
|
||
| val observer: MutableLiveData<Int> |
There was a problem hiding this comment.
Тут есть проблема, у тебя сейчас в обоих фрагментах торчит MutableLiveData, то есть ты можешь заэмитить что-то в MutableLiveData даже из RecieverViewModel, который предполагается умеет только читать. Попробуй прокинуть туда экземпляр LiveData, а в ProducerViewModel прокинь MutableLiveData
| import java.lang.RuntimeException | ||
| import javax.inject.Inject | ||
|
|
||
| class ViewModelProducer @Inject constructor ( |
| import javax.inject.Inject | ||
|
|
||
| class ViewModelProducer @Inject constructor ( | ||
| private val colorGenerator: ColorGenerator?, |
2 participants
No description provided.