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

SafeArgs導入 #625

Merged
merged 4 commits into from Jan 25, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+12 −15
Diff settings

Always

Just for now

レビュー指摘対応

  • Loading branch information...
sobaya-0141 committed Jan 25, 2019
commit f98dd3f4d91803e540786f0dcc3d951aedbb5a9b
@@ -40,6 +40,10 @@ class TabularFormSessionPageFragment : DaggerFragment() {
InjectedViewModelProviders.of(requireActivity()).get(sessionPagesStoreProvider)
}

private val args: TabularFormSessionPagesFragmentArgs by lazy {

This comment has been minimized.

@tomoya0x00

tomoya0x00 Jan 25, 2019

Contributor

すみません。この方法なのですが、Fragmentのインスタンスは再利用されることがあるので、
バグの原因になることがありそうです。

下記をご参照。
https://www.slideshare.net/RecruitLifestyle/kotlin-87339759
P.69

我々からの指摘なのに、すみません 🙇

This comment has been minimized.

@tomoya0x00

tomoya0x00 Jan 25, 2019

Contributor

例えば lateinit として定義していただき、onActivityCreatedで毎回初期化 が良さそうです。

This comment has been minimized.

@tomoya0x00

tomoya0x00 Jan 25, 2019

Contributor

English version:

So sorry 🙇
The method of initializing properties using lazy in Fragment is not good,
Because instance of Fragment may be reused 😇

What do you think of using lateinit instead?

This comment has been minimized.

@sobaya-0141

sobaya-0141 Jan 25, 2019

Author Contributor

この資料は以前読んだのに失念しておりました。
修正したので、ご確認お願いします。

TabularFormSessionPagesFragmentArgs.fromBundle(arguments ?: Bundle())
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
@@ -91,12 +95,7 @@ class TabularFormSessionPageFragment : DaggerFragment() {
adapter = groupAdapter
}

val day = if (arguments != null) {
TabularFormSessionPagesFragmentArgs.fromBundle(arguments!!).day
} else {
1
}
sessionPagesStore.sessionsByDay(day)
sessionPagesStore.sessionsByDay(args.day)
.changed(viewLifecycleOwner) { sessions ->
groupAdapter.update(fillGaps(sessions))
}
@@ -168,15 +167,9 @@ class TabularFormSessionPageFragment : DaggerFragment() {
companion object {
const val COLUMN_COUNT = 9

fun newInstance(day: Int): TabularFormSessionPageFragment {
fun newInstance(args: TabularFormSessionPagesFragmentArgs): TabularFormSessionPageFragment {
return TabularFormSessionPageFragment()
.apply {
arguments =
TabularFormSessionPagesFragmentArgs.Builder()
.setDay(day)
.build()
.toBundle()
}
.apply { arguments = args.toBundle() }
}
}
}
@@ -55,7 +55,11 @@ class TabularFormSessionPagesFragment : DaggerFragment() {
private val days = listOf(SessionPage.pageOfDay(1), SessionPage.pageOfDay(2))

override fun getItem(position: Int): Fragment {
return TabularFormSessionPageFragment.newInstance(days[position].day)
return TabularFormSessionPageFragment.newInstance(
TabularFormSessionPagesFragmentArgs.Builder()
.setDay(days[position].day)
.build()
)
}

override fun getPageTitle(position: Int) = days[position].title
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.