Tukukkk

Currently, I have a FragmentPagerAdapter whose getItem() method looks like this:

private static String makeFragmentName(int viewId, int index) {

    return "android:switcher:" + viewId + ":" + index;

}



@Override

public Fragment getItem(int position) {

    String fragmentName = makeFragmentName(R.id.rec_or_gallery_viewpager, position);

    Fragment fragment = fm.findFragmentByTag(fragmentName);

    Log.i("test_fragment_inst", fragment+"");

    if( position == 0 ) {

        if(fragment == null) {

            fragment = PicmixVideoGalleryChooserFragment.newInstance();

        }

    } else {

        if(fragment == null) {

            fragment = PicmixVideoRecordFragment.newInstance();

        }

    }

    return fragment;

}

I was thinking of changing it into this:

private Map<Integer, Fragment> fragmentMap;



public RecordOrGalleryFragmentPagerAdapter(FragmentManager fm) {

    super(fm);

    this.fm = fm;

    fragmentMap = new HashMap<Integer, Fragment>();

}



...



@Override

public Fragment getItem(int position) {

    Fragment fragment = fragmentMap.get(position);

    Log.i("test_fragment_inst", fragment+"");

    if( position == 0 ) {

        if(fragment == null) {

            fragment = PicmixVideoGalleryChooserFragment.newInstance();

            fragmentMap.put(position, fragment);

        }

    } else {

        if(fragment == null) {

            fragment = PicmixVideoRecordFragment.newInstance();

            fragmentMap.put(position, fragment);

        }

    }

    return fragment;

}

Both do the same thing, basically, so will there be any performance improvements if I use the latter method, or should I just use the former method?

Assuming you have the first variant, where you are using FragmentManager to access Fragments from the stack. It's possible to retrieve this fragment, if it was created before.

Ex. one screen doesn't have Pager, but few Fragments there, later you switched to screen two, where located your FragmentPager, and you are using previously created Objects of Fragments. However, you can do the same with shared Collection of Fragments (what I don't recommend you to do).

At the same time, it's just common behavior to locate Collection of Objects in Adapter pattern implementation (in your case FragmentPager and his Adapter). And there is no performance difference of using one, or another solution.

One more note, you can even simplify Map to simple Array. And access to the Fragment Objects with their position indexes.

With your approach, it is unsafe to use a Map to store and retrieve your fragments by position, because they may not survive configuration changes, and may throw unexpected errors such as IllegalStateException when you attempt to access them later.

Although having tags on fragments seems to be a better choice, but if your adapter is extending to FragmentPagerAdapter or FragmentStatePagerAdapter, then it is a bad idea. In that case, you should not be returning old fragments in your getItem.

Here's a snippet of what FragmentPagerAdapter.instantiateItem does:

@NonNull

public Object instantiateItem(@NonNull ViewGroup container, int position) {

    if (this.mCurTransaction == null) {

        this.mCurTransaction = this.mFragmentManager.beginTransaction();

    }



    long itemId = this.getItemId(position);

    String name = makeFragmentName(container.getId(), itemId);

    Fragment fragment = this.mFragmentManager.findFragmentByTag(name);

    if (fragment != null) {

        this.mCurTransaction.attach(fragment);

    } else {

        fragment = this.getItem(position); // Get new instance of fragment

        this.mCurTransaction.add(container.getId(), fragment, makeFragmentName(container.getId(), itemId));

    }



    if (fragment != this.mCurrentPrimaryItem) {

        fragment.setMenuVisibility(false);

        fragment.setUserVisibleHint(false);

    }



    return fragment;

}

It is absolutely clear that you should always return a new instance of fragment in getItem, or you will end up doing the same thing as instantiateItem. The ViewPager uses instantiateItem to decide how it should render pages, so if it needs a new one, your getItem better be returning a new instance.