-
Notifications
You must be signed in to change notification settings - Fork 174
Выполнение всех заданий #62
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
base: master
Are you sure you want to change the base?
Conversation
antonkazakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Привет. Поправь замечания, пожалуйста
|
|
||
| @Inject | ||
| @ActivityContext | ||
| lateinit var activityContext: Context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Его можно через requireContext запросить, не вижу смысла инжектить
| lateinit var activityContext: Context | ||
|
|
||
| lateinit var fragmentProducerComponent: FragmentProducerComponent | ||
| private val colorGenerator = ColorGeneratorImpl() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А его как раз нужно инжектить во вьюмодель
| @ActivityContext | ||
| lateinit var activityContext: Context | ||
|
|
||
| lateinit var fragmentProducerComponent: FragmentProducerComponent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем тебе ссылка на компонент на уровне класса?
| ): View? { | ||
| return inflater.inflate(R.layout.fragment_a, container, true) | ||
| val activityComponent = (requireActivity() as MainActivity).activityComponent | ||
| fragmentProducerComponent = DaggerFragmentProducerComponent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Лучше его создавать в onCreate/onAttach, сейчас он будет пересоздаваться каждый раз когда фрагмент будет класться/доставаться из бекстека
| import javax.inject.Provider | ||
|
|
||
|
|
||
| class FragmentReceiver : Fragment() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Аналогичные замечания
| import javax.inject.Inject | ||
|
|
||
|
|
||
| @ActivityScope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь не нужен скоуп
| fun getFlow(): StateFlow<Int> | ||
| } | ||
|
|
||
| @ActivityScope |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Здесь тоже. Скоуп нужен если ты будешь инжектить конкретно этот тип, а ты инжектишь IDataKeeper и биндишь его помечая скоупом в даггеровском модуле
| fun inject(fragmentReceiver: FragmentReceiver) | ||
|
|
||
| @ApplicationContext | ||
| fun provideApplicationContext(): Context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем тебе провижен метод во фрагмент компоненте? Кто-то будет зависить от FragmentReceiverComponent ?
| fun inject(fragmentProducer: FragmentProducer) | ||
|
|
||
| @ApplicationContext | ||
| fun provideApplicationContext(): Context |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Все эти провижен методы не нужны
| lateinit var activityComponent: ActivityComponent | ||
|
|
||
| @Inject | ||
| lateinit var dataKeeper: Provider<IDataKeeper> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Разве он нужен здесь?
antonkazakov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
No description provided.