2
\$\begingroup\$

How could I simplify the onCreate() method? Maybe via compact and properly-named methods?

public class FilterActivity extends FragmentActivity {    
    private Bundle args;
    private FilterFragment filterFragment;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_3_filter);

        if (savedInstanceState == null) {
            args = getIntent().getExtras();
        } else {
            args = savedInstanceState.getBundle(Keys.args);
        }

        FragmentManager fragmentManager = getSupportFragmentManager();
        FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
        filterFragment = new FilterFragment();
        filterFragment.setArguments(args);
        if (savedInstanceState == null) {
            fragmentTransaction.add(R.id.containerOfFilterFragment, filterFragment);
        } else {
            fragmentTransaction.replace(R.id.containerOfFilterFragment, filterFragment);
        }
        fragmentTransaction.commit();

        filterFragment = (FilterFragment)
                getSupportFragmentManager().findFragmentById(R.id.containerOfFilterFragment);
    }

    @Override
    protected void onSaveInstanceState(Bundle outState) {
        super.onSaveInstanceState(outState);
        outState.putBundle(Keys.args, args);
    }
}
\$\endgroup\$
3
\$\begingroup\$

Is this method really a problem? Why are you worried about it? There are some small changes you can make, but they do not have much of an impact on the performance of the routine (if it is slow...).

About the only things I can think of improving are:

  1. you check whether the savedInstanceState is null twice, and those two checks can be merged in to one...
  2. you create a new FilterFragment instance, and you either add it to (if it is not there), or replace it (if there already was one). Thus, you have the filterFragement, and you do not need to findFragmentById(...) since you should be finding the fragment you just put in there....

Here's your method with the lines commented out that I don't think you need, and also two new lines marked with //**ADDED**

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_3_filter);

    //if (savedInstanceState == null) {
    //    args = getIntent().getExtras();
    //} else {
    //    args = savedInstanceState.getBundle(Keys.args);
    //}

    FragmentManager fragmentManager = getSupportFragmentManager();
    FragmentTransaction fragmentTransaction = fragmentManager.beginTransaction();
    filterFragment = new FilterFragment();
    //filterFragment.setArguments(args);
    if (savedInstanceState == null) {
        filterFragment.setArguments(getIntent().getExtras());  //**ADDED**
        fragmentTransaction.add(R.id.containerOfFilterFragment, filterFragment);
    } else {
        filterFragment.setArguments(savedInstanceState.getBundle(Keys.args));  //**ADDED**
        fragmentTransaction.replace(R.id.containerOfFilterFragment, filterFragment);
    }
    fragmentTransaction.commit();

    //filterFragment = (FilterFragment)
    //        getSupportFragmentManager().findFragmentById(R.id.containerOfFilterFragment);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.