Ali
Ali

Reputation: 9994

Avoid passing a parameter multiple times in fragment

I am using ViewPager2 in my app. I have following code to set adapter to ViewPager :

viewModel.creditWrapper.observe(viewLifecycleOwner, Observer { creditWrapper ->
                fragmentManager?.let {
                    val myAdapter = ViewPagerFragmentAdapter(it, lifecycle)

                    val castPagerItem = PagerItem(CreditFragment.newInstance(tmdbItem,
                            creditWrapper.cast as ArrayList<Cast>), R.string.cast)
                    val crewPagerItem = PagerItem(CreditFragment.newInstance(tmdbItem,
                            creditWrapper.crew as ArrayList<Crew>), R.string.crew)

                    myAdapter.addFragment(castPagerItem)
                    myAdapter.addFragment(crewPagerItem)

                    pager.adapter = myAdapter
                    TabLayoutMediator(tab_layout, pager) { tab, position ->
                        tab.text = getString(myAdapter.creditFragments[position].title)
                    }.attach()
                }
            })

As you see I pass tmdbItem multiple times as a parameter to my newInstance() method. Is there any solution to avoid that and do not pass a parameter multiple times?

Here is my CreditFragment :

class CreditFragment<T : Credit> : Fragment() {

    companion object {
        private const val TMDB_ITEM = "tmdb_item"
        private const val CREDITS = "credits"

        @JvmStatic
        fun <T : Credit> newInstance(item: TmdbItem, credits: ArrayList<T>) =
                CreditFragment<T>().apply {
                    arguments = Bundle(2).apply {
                        putParcelable(TMDB_ITEM, item)
                        putParcelableArrayList(CREDITS, credits)
                    }
                }
    }

    override fun onCreateView(inflater: LayoutInflater, container: ViewGroup?,
                              savedInstanceState: Bundle?): View? {

        val root = inflater.inflate(R.layout.credit_fragment, container, false)
        root.credit_list.apply {
            setHasFixedSize(true)
            adapter = arguments?.getParcelableArrayList<T>(CREDITS)?.let {
                CreditAdapter(it, object : CreditClickCallback {
                    override fun onClick(personId: Any, personName: String, profilePath: String?) {
                        val intent = Intent(activity, PersonActivity::class.java).apply {
                            putExtras(Bundle().apply {
                                putParcelable(PERSON_WRAPPER, PersonWrapper(personId,
                                        personName,
                                        profilePath,
                                        arguments?.getParcelable<TmdbItem>(TMDB_ITEM)?.backdropPath))
                            })
                        }
                        startActivity(intent)
                    }
                })
            }
        }
        return root
    }
}

Full source code can be found at : https://github.com/AliRezaeiii/TMDb-Paging

Upvotes: 1

Views: 154

Answers (2)

Maithili Joshi
Maithili Joshi

Reputation: 134

If you want to absolutely avoid duplication (though its ok in this case), then you can create a companion object of the Parent Fragment which holds the ViewPager tabs. companion object { var tmdbItem: TmdbItem } Store the tmdbItem in that object. You can directly refer this from child view pager fragments like ParentFragment.tmdbItem

Upvotes: 0

Mariusz Brona
Mariusz Brona

Reputation: 1609

In my opinion your code is okay. I see that you have a lot of code moved to abstract/parent classes. If it's okay for you, then I think there's not much you can do. With this approach you are creating a lot of coupling and you loose flexibility, but as I said - if it works for you and you know you won't need it, then it's okay.

You are using factory method for passing arguments which is a good approach, and for me it's totally acceptable. If you are trying to go more and more generic and your goal is to have as little duplicated code as possible, then as I said - you can loose flexibility.

So the tmdbItem can be either retreived from Fragment/Activity, or the lower layers of your app. Of course you can inject some kind of TmdbItemProvider with a DI framework, but I don't think it's a good idea, because you can have a bottleneck there and loose control over the state of tmdbItem.

For me it's okay and a little duplication never killed nobody ;)

And I will also consider looking at the topic of Composition over Inheritance

Upvotes: 1

Related Questions