3
\$\begingroup\$

My current code is kinda atrocious to switch fragments and everything, but I don't know a better way to check to see if the fragment I'm trying to switch to.

private void FragmentChange(Bundle Data) {
    SetActionBarButtons(new String[] { "SearchButtonOff", "SearchExtraButtonOff" });
    String[] data = Data.getStringArray(ACTIVITY_MAIN.DATA);
    String type = data[0];
    String fragment = data[1];
    if (fragment.equals(POST_MAIN)) {
        postmainFragment = new Fragment_PostMain();
        mainContainer = postmainFragment;
    } else if (fragment.equals(POST_ACCOUNT)) {
        postaccountFragment = new Fragment_PostAccount();
        mainContainer = postaccountFragment;
    } else if (fragment.equals(POST_ACCOUNTLOGIN)) {
        postaccountloginFragment = new Fragment_PostAccountLogin();
        mainContainer = postaccountloginFragment;
    } else if (fragment.equals(POST_LOCATION_MAIN)) {
        postlocationmainFragment = new Fragment_PostLocationMain();
        mainContainer = postlocationmainFragment;
    } else if (fragment.equals(POST_IMAGES)) {
        postimagesFragment = new Fragment_PostImages();
        mainContainer = postimagesFragment;
    } else if (fragment.equals(POST_TEMPLATE)) {
        posttemplateFragment = new Fragment_PostTemplate();
        mainContainer = posttemplateFragment;
    } else if (fragment.equals(POST_LOCATION)) {
        postlocationFragment = new Fragment_PostLocation();
        mainContainer = postlocationFragment;
    } else if (fragment.equals(POST_CATEGORY)) {
        postcategoryFragment = new Fragment_PostCategory();
        mainContainer = postcategoryFragment;
    } else if (fragment.equals(POST_CATEGORY_MAIN)) {
        postcategorymainFragment = new Fragment_PostCategoryMain();
        mainContainer = postcategorymainFragment;
    } else if (fragment.equals(POST_LOCATION_MAIN)) {
        postlocationmainFragment = new Fragment_PostLocationMain();
        mainContainer = postlocationmainFragment;
    } else if (fragment.equals(LOADING)) {
        loadingFragment = new Fragment_Loading();
        mainContainer = loadingFragment;
    } else if (fragment.equals(POST_RESULTS_IMAGEVIEW)) {
        postresultsimageviewFragment = new FRAGMENT_POST_RESULTS_IMAGEVIEW();
        mainContainer = postresultsimageviewFragment;
    } else if (fragment.equals(SEARCH_RESULTS_REPLY)) {
        postresultsreplyFragment = new FRAGMENT_SEARCH_RESULTS_REPLY();
        mainContainer = postresultsreplyFragment;
    }


    FragmentTransaction FT = getFragmentManager().beginTransaction();
    if (type.equals(FRAGMENTCHANGE_FADEIN)) {
        FT.setCustomAnimations(R.anim.fade_in, R.anim.fade_out);
    } else if (type.equals(FRAGMENTCHANGE_FLIP_FORWARD)) {
        FT.setCustomAnimations(R.animator.card_flip_right_in, R.animator.card_flip_right_out);
    } else if (type.equals(FRAGMENTCHANGE_FLIP_BACK)) {
        FT.setCustomAnimations(R.animator.card_flip_left_in, R.animator.card_flip_left_out);
    }
    if (mainContainer != null) {
        FT.replace(R.id.main_fragment, mainContainer).commit();
    }
}

Basically, I want to pass a Bundle down and I set the first part of the bundle as the "type" for the animation, and the fragment as the actual fragment that I'm trying to switch it. Then of course since I have so many fragments it just gets really messy with the if statements.

Is there a better way to write this so it's not as "ugly"?

\$\endgroup\$
  • \$\begingroup\$ Have you tried using a switch statement? \$\endgroup\$ – DacorieS Sep 15 '15 at 23:05
2
\$\begingroup\$

Here's few things to avoid "ugly" dups and long-long methods:

Don't duplicate variables

Like your mainContainer variable, because you used one variable for each fragment and pass it to another global variable:

postmainFragment = new Fragment_PostMain(); // global variable outside fragmentChange()
mainContainer = postmainFragment; // global variable outside fragmentChange()
...
postaccountFragment = new Fragment_PostAccount();
mainContainer = postaccountFragment;
...

It should be better to use just one variable, and you will avoid duplicate allocations:

Fragment mainContainer = null; // inside fragmentChange()
...
mainContainer = new Fragment_PostMain(); // directly set the fragment
...
mainContainer = new Fragment_PostAccount();

Therefore, you will not create global fragments variables for each fragments in parent class (postmainFragment, postaccountFragment, etc).

Use "switch" as @dacories said

You can do with Integers instead of Strings (it might be faster), for example:

private final static int FRAGMENTCHANGE_FADEIN = 0;
private final static int FRAGMENTCHANGE_FLIP_FORWARD = 1;
private final static int FRAGMENTCHANGE_FLIP_BACK= 2;
...
private void fragmentChange(Bundle data) {
    int transition = data[0];
    switch(transition) {
        case FRAGMENTCHANGE_FADEIN:
             ...
        case FRAGMENTCHANGE_FLIP_FORWARD:
             ...
    }
}

You can do the same for fragments...

Do a general method and avoid long-long switch

Instead of passing an array and switching between all your fragment in the fragmentChange method, you could pass directly the fragment concerned:

private void fragmentChange(Fragment frag, String tag, int transition) {
    if (frag != null) {
        ...
        FragmentTransaction ft = getFragmentManager().beginTransaction();
        switch (transition) {
            default:
            case FRAGMENTCHANGE_FADEIN:
                ft.setCustomAnimations(R.anim.fade_in, R.anim.fade_out);
                break;
            case FRAGMENTCHANGE_FLIP_FORWARD:
                ft.setCustomAnimations(R.animator.card_flip_right_in, 
                                    R.animator.card_flip_right_out);
                break;
            case FRAGMENTCHANGE_FLIP_BACK:
                ft.setCustomAnimations(R.animator.card_flip_left_in, 
                                    R.animator.card_flip_left_out);
                break;
        }
        ft.replace(R.id.main_fragment, frag, tag).commit();
    }
}

So, you could directly call the change method as follows:

fragmentChange(new Fragment_PostMain(), "PostMain", 0);

You will in this case avoid a global fragment variable mainContainer which is constantly allocated in the whole class and avoid a very long switch or if/else condition.

But, if I want to pass datas to the next fragment?

One solution could be using static instance methods to pass datas with setArguments. You could have an instance in fragment class to attach the datas and return just the fragment into the fragmentChange method. An example might be in each fragment:

public static Fragment_PostMain newInstance(int i, String n, boolean b) {
    Fragment_PostMain frag = new Fragment_PostMain();
    Bundle datas = new Bundle();
    datas.putInt(i);
    datas.putString(n);
    datas.putBoolean(b);
    frag.setArguments(datas);
    return frag;
}  

You could retrieve these datas with getArguments methods in fragment.
And call the method by passing the fragment with datas:

Fragment_PostMain frag = Fragment_PostMain.newInstance(12, "NameSection", true);
fragmentChange(frag, "PostMain", 0);

Like this, you won't have to create a Bundle array in the method.

Finally

Your solution with long-long if/else can be more readable with switch statement, yes. My solutions above will be more readable with less code, yes again.
However, you will have to think of what you really need: a long condition code (which can be easier to change in future if one fragment change for all class) or a short version (with class customization and direct access to the datas). I hope this will help you.

Just an advice: in Java, the standard naming conventions is Classes should begin with an uppercase whereas methods with lowercase.

\$\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.