Refactoring Android fragments

Question

I have an Android app that shows some fragments, but I'm repeating lots of code.

MainActivity.java

public class MainActivity extends FragmentActivity {
    MyPagerAdapter pageAdapter;
    ViewPager pager;
    @Override

    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        pageAdapter = new MyPagerAdapter(getSupportFragmentManager());
        pager = (ViewPager)findViewById(R.id.myViewPager);
        pager.setAdapter(pageAdapter);
    }

    public void homeButtonPressed(View v) {
        // go home
        pager.setCurrentItem(0);  
    }
}

MyPagerAdapter.java

public class MyPagerAdapter extends FragmentPagerAdapter {
    public List<Fragment> fragments;

    public MyPagerAdapter(FragmentManager fm) {
        super(fm);
        this.fragments = new ArrayList<Fragment>();
        fragments.add(new Page1());
        fragments.add(new Page2());
        fragments.add(new Page3());
        fragments.add(new Page4());
        fragments.add(new Page5());
        fragments.add(new Page6());
        fragments.add(new Page7());
        fragments.add(new Page8());
    }

    @Override
    public Fragment getItem(int position) {
        return fragments.get(position);
    }

    @Override
    public int getCount() {
        return fragments.size();
    }
}

I'm loading in the page adapter the fragments to show, but they are all the same with a few differences.

Layout for page 1

<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:background="#F7F4E7">
    <ImageView
        android:id="@+id/imageView1"
        android:layout_width="match_parent"
        android:layout_height="match_parent"
        android:layout_alignParentLeft="true"
        android:layout_alignParentTop="true"
        android:src="@drawable/page_1" />
</RelativeLayout>

Page1.java

public class Page1 extends Fragment {

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {

        View view = inflater.inflate(R.layout.page1_layout, container, false);
        return view;
    }
}

The only difference on each page is the

android:src="@drawable/page_1"

and showing a button or hiding it for certain fragments.

I come from Objective-C, and I understand that I need to abstract this, as I have created all of this classes PageN, that are the same! ,but I don't yet understand how to load them on the pagerAdapter .

Answer 1

The main problem is the difficulty of abstracting resources. After compilation, they can only be accessed by their unqiue identifiers, accessible through the generated class R which you already use: R.layout.page1_layout.

Step 1

Let's be strict about the well-known concept DRY: If all of your PageN classes share the same content and the only difference is the referenced layout, you can replace all of your classes PageN with this single class:

public class PageFragment extends Fragment {

    private int layout;

    public PageFragment(int layout) {
        this.layout = layout;
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        View view = inflater.inflate(this.layout, container, false);
        return view;
    }
}

You can now use this code to initialize the fragments:

fragments.add(new PageFragment(R.layout.page1_layout));
fragments.add(new PageFragment(R.layout.page2_layout));
fragments.add(new PageFragment(R.layout.page3_layout));
// etc...

Step 2

If the layout files differ only in a resource and the visibility of a button, you should abstract it further. Let's think about how to structure the information: All we need in our fragment is the layout file (which should always be the same), the @drawable resource and a boolean for the button visibility.

Let's assume you created a new layout file containing the image view with android:id="myImageView" and the button with android:id="myButton". Everything else should be equal from page to page.

public class PageFragment extends Fragment {

    private int drawable;
    private boolean showButton;

    public PageFragment(int drawable, boolean showButton) {
        this.drawable= drawable;
        this.showButton = showButton;
    }

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        // Always the same layout
        View view = inflater.inflate(R.layout.page_layout, container, false);
        // Apply per-page configuration
        ((ImageView) view.findViewById(R.id.myImageView)).setImageResource(this.drawable);
        if(!this.showButton) {
            view.findViewById(R.id.myButton).setVisibility(View.INVISIBLE);
        }
        return view;
    }
}

The initialization code is not really pretty:

fragments.add(new PageFragment(R.drawable.page1, true));
fragments.add(new PageFragment(R.drawable.page2, false));
fragments.add(new PageFragment(R.drawable.page3, true));
// etc...

Step 3

Last but not least, you could abstract the initialization. That's not as simple as the previous steps and not necessarily an improvement. Depending on the total number of pages, you might want to keep the initialization code from the second step. It's actually a commonly used practice to avoid abstraction in similar situations, in other words, when the effort of storing the data in an appropriate form is unjustifiable.

In my opinion, it might be a good idea not to call fragments.add in every single line (although it causes a little overhead).

PageFragment[] fragments = {
    new PageFragment(R.drawable.page1, true),
    new PageFragment(R.drawable.page2, false),
    new PageFragment(R.drawable.page3, true)
};
for(PageFragment fragment : fragments) {
    this.fragments.add(fragment);
}

You already declared fragments as a List<Fragment>, so this code should work as well:

PageFragment[] fragments = {
    new PageFragment(R.drawable.page1, true),
    new PageFragment(R.drawable.page2, false),
    new PageFragment(R.drawable.page3, true)
};
this.fragments = Arrays.asList(fragments);

Strictly following DRY ("Every piece of knowledge must have a single, unambiguous, authoritative representation within a system"), this approach should be preferred over the last ones.

However, the most important steps are the first ones, don't waste time trying to optimize the initialization.