0

I personally find it easier to use and read switch case scenarios. Does anyone know what this code for my list view can be changed to so that it utilises switch statements on strings instead of if statements? I've changed the compiler to 1.7 as necessary.

@Override
    public void onActivityCreated(Bundle savedInstanceState) {
        View v = getView();

        if (getActivity().findViewById(R.id.detail_container) != null) {
            mTwoPane = true;
        } else {
            mTwoPane = false;
        }

        ListView lv = (ListView)v.findViewById(android.R.id.list);
        lv.setChoiceMode(ListView.CHOICE_MODE_SINGLE);

        lv.setOnItemClickListener(new AdapterView.OnItemClickListener() {



@Override
            public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
                // get the adapter, then get the name from the adapter at that position
        WorldListAdapter adapter = (WorldListAdapter) parent.getAdapter();
        String country = adapter.getItem(position);

                if (mTwoPane) {
            setItemNormal();
            View rowView = view;
            setItemSelected(rowView);

            Fragment newFragment;
            if (country.equals(view.getResources().getString(R.string.africa))) {
                newFragment = new FragmentAfrica();
            } else if (country.equals(view.getResources().getString(R.string.asia))) {
                newFragment = new FragmentAsia();
            } else if (country.equals(view.getResources().getString(R.string.europe))) {
                newFragment = new FragmentEurope();
            } else {
                newFragment = new FragmentAfrica();
            }
            WorldActivity activity = (WorldActivity) view.getContext();
            FragmentTransaction transaction = activity.getSupportFragmentManager().beginTransaction();
            transaction.replace(R.id.detail_container, newFragment);
            transaction.commit();
        } else {
            Intent intent;
            if (country.equals(view.getResources().getString(R.string.africa))) {
                intent = new Intent(getActivity(), AfricaActivity.class);
            } else if (country.equals(view.getResources().getString(R.string.asia))) {
                intent = new Intent(getActivity(), AsiaActivity.class);
            } else if (country.equals(view.getResources().getString(R.string.europe))) {
                intent = new Intent(getActivity(), EuropeActivity.class);
            } else {
                intent = new Intent(getActivity(), AfricaActivity.class);
            }
            startActivity(intent);
        }
    }

            public void setItemSelected(View view) {
                View rowView = view;
                view.setBackgroundColor(Color.parseColor("#1C3F96"));

                TextView tv0 = (TextView) rowView.findViewById(R.id.country);
                tv0.setTextColor(Color.parseColor("#FFFFFF"));

                TextView tv1 = (TextView) rowView.findViewById(R.id.country_description);
                tv1.setTextColor(Color.parseColor("#FFFFFF"));
            }

            public void setItemNormal() {
                for (int i = 0; i < getListView().getChildCount(); i++) {
                    View v = getListView().getChildAt(i);
                    v.setBackgroundColor(Color.TRANSPARENT);

                    TextView tv0 = ((TextView) v.findViewById(R.id.country));
                    tv0.setTextColor(Color.WHITE);

                    TextView tv1 = ((TextView) v.findViewById(R.id.country_description));
                    tv1.setTextColor(Color.parseColor("#B5B5B5"));
                }
            }
        });

        super.onActivityCreated(savedInstanceState);
    }
wbk727
  • 8,017
  • 12
  • 61
  • 125
  • I think this question belongs on code review – Jay Harris Jul 12 '15 at 00:34
  • 1
    What if `switch` statements aren't the best solution? – Mathieu Guindon Jul 12 '15 at 00:36
  • I would suggest branching on multiple levels as in your example are better handled with methods that deal with each individual set of if/else sections (not combining them all into a complex switch statement) ... Also, eliminate if/else where you can and prefer Factory pattern – Constantin Jul 12 '15 at 00:38
  • I don't think this would be a good fit for Code Review as worded. Sounds like OP is trying to change the functionality of their code. Once the code functions _as intended_, they welcome to post it on CR for peer review. – Phrancis Jul 12 '15 at 00:38
  • 1
    @Phrancis no, OP isn't asking to change the functionality, just how it's done (`if` vs. `switch`) - I think it would be on-topic on Code Review, but I would prefer seeing the full methods for full context (mismatching braces indicate a chunk was cut off), so as to better allow reviewers to [comment on any and all facets of the code](http://www.codereview.stackexchange.com/help/on-topic). I would also advise to get the indentation to look exactly as OP has it in their IDE. – Mathieu Guindon Jul 12 '15 at 00:41

2 Answers2

2

java 8 supports switch case with String

 @Override
public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
    // get the adapter, then get the name from the adapter at that position
    WorldListAdapter adapter = (WorldListAdapter) parent.getAdapter();
    String country = adapter.getItem(position);

    if (mTwoPane) {
        setItemNormal();
        View rowView = view;
        setItemSelected(rowView);

        Fragment newFragment;
        switch (country.toLowerCase()) {
            case "africa":
                newFragment = new FragmentAfrica();
                break;
            case "asia":
                newFragment = new FragmentAsia();
                break;
            case "europe":
                newFragment = new FragmentEurope();
                break;
            default:
                newFragment = new FragmentAfrica();
        }
        WorldActivity activity = (WorldActivity) view.getContext();
        FragmentTransaction transaction = activity.getSupportFragmentManager().beginTransaction();
        transaction.replace(R.id.detail_container, newFragment);
        transaction.commit();
    } else {
        Intent intent;
        switch (country.toLowerCase()) {
            case "africa":
                intent = new Intent(getActivity(), AfricaActivity.class);
                break;
            case "asia":
                intent = new Intent(getActivity(), AsiaActivity.class);
                break;
            case "europe":
                intent = new Intent(getActivity(), EuropeActivity.class);
                break;
            default:
                intent = new Intent(getActivity(), AfricaActivity.class);
        }

        startActivity(intent);
    }
}
           you can replace the literals `africa`,`asia`,`europe` with anything you want
Kainix
  • 1,186
  • 3
  • 21
  • 33
  • Cool. Although why is there a `default:` there? – wbk727 Jul 12 '15 at 12:45
  • coz in your `if-else-if` ladder if none of the `String` matches you'd initialize the `newFragment` with `FragmentAfrica` I thought that its a default case thats why I added it. – Kainix Jul 12 '15 at 13:20
  • How can I use strings instead of hard coding? – wbk727 Jul 16 '15 at 13:05
  • We use `switch-case` when we've fixed options to deal wtih, if you're thinking of replacing the literals viz. 'asia','europe' etc with a variable you're basically implementing 'if-else' using 'switch-case` – Kainix Jul 16 '15 at 13:28
  • I presume by harding coding you are taking about those cases,right? – Kainix Jul 16 '15 at 13:29
  • Actually starting with java 7 there is support for switch case with String – andrei Jul 16 '15 at 13:32
  • oh..my bad..but that doesn't answer why you don't want those literals – Kainix Jul 16 '15 at 13:47
  • I meant string resources – wbk727 Jul 16 '15 at 15:23
  • sorry bro Android ain't my expertise you might want to ask another question on SO with this regard – Kainix Jul 16 '15 at 16:12
  • Hey its okay if you unchecked it as 'answer' but you're asking different question in the comment section. I did answer your question though. – Kainix Aug 01 '15 at 12:29
  • Not saying for rep, but others who come across this on SO will think the question is unanswered whereas you actually question lies somewhere deep in the comments section.. I recommend you either update your question or post another one, peace out – Kainix Aug 01 '15 at 12:30
1

Create and use Factory methods instead ..

Fragment newFragment = FragmentFactory.createInstance(country, view);
...            
Intent intent = IntentFactory.createInstance(country, view);

This way, you can add new types of Fragments and Intents in your Factory classes but leave the types generic in client classes. It is more maintainable and extensible this way

Consider this post for another take on this topic Simple Factory vs Factory Method: Switch statement in factory vs. client

Here is a possible Factory example for your case

public class FragmentFactory {

    public static Fragment getInstance(String country, View view) {
        Fragment newFragment = null;

        if (country.equals(view.getResources().getString(R.string.africa))) {
            newFragment = new FragmentAfrica();
        } 
        else if (country.equals(view.getResources().getString(R.string.asia))) {
            newFragment = new FragmentAsia();
        } 
        else {
            if (country.equals(view.getResources().getString(R.string.europe))) {
                newFragment = new FragmentEurope();
            }
        } 
        return  newFragment;       
    }
}

You can try it as a switch

public static Fragment getInstance(String country, View view, ) {
    Fragment newFragment = null;

    switch (country.toLowerCase()) {

        case view.getResources().getString("africa"):
            newFragment = new FragmentAfrica();
            break;

        case view.getResources().getString("asia"):
            newFragment = new FragmentAsia();
            break;

        case view.getResources().getString("europe"):
            newFragment = new FragmentEurope();
            break;
    }               
    return newFragment;
}
Community
  • 1
  • 1
Constantin
  • 1,506
  • 10
  • 16