1
\$\begingroup\$

I have this 2 arrays :

final int[] tabSelected = {
        R.drawable.iconWhite0,
        R.drawable.iconWhite1,
        R.drawable.iconWhite2,
        R.drawable.iconWhite3,
        R.drawable.iconWhite4};
final int[] tabsUnselected = {
        R.drawable.iconBlack0,
        R.drawable.iconBlack1,
        R.drawable.iconBlack2,
        R.drawable.iconBlack3,
        R.drawable.iconBlack4};

I just don't understand how I can simplify this, by making a for loop ..although I am sure it is very easy, I just don't see it.

  @Override
        public void onPageSelected(int position) {
            switch (position) {
                case 0:
                    tabLayout.getTabAt(0).setIcon(tabSelected[0]);
                    tabLayout.getTabAt(1).setIcon(tabsUnselected[1]);
                    tabLayout.getTabAt(2).setIcon(tabsUnselected[2]);
                    tabLayout.getTabAt(3).setIcon(tabsUnselected[3]);
                    tabLayout.getTabAt(4).setIcon(tabsUnselected[4]);
                    break;

                case 1:
                    tabLayout.getTabAt(0).setIcon(tabsUnselected[0]);
                    tabLayout.getTabAt(1).setIcon(tabSelected[1]);
                    tabLayout.getTabAt(2).setIcon(tabsUnselected[2]);
                    tabLayout.getTabAt(3).setIcon(tabsUnselected[3]);
                    tabLayout.getTabAt(4).setIcon(tabsUnselected[4]);
                    break;

                case 2:
                    tabLayout.getTabAt(0).setIcon(tabsUnselected[0]);
                    tabLayout.getTabAt(1).setIcon(tabsUnselected[1]);
                    tabLayout.getTabAt(2).setIcon(tabSelected[2]);
                    tabLayout.getTabAt(3).setIcon(tabsUnselected[3]);
                    tabLayout.getTabAt(4).setIcon(tabsUnselected[4]);
                    break;

                case 3:
                    tabLayout.getTabAt(0).setIcon(tabsUnselected[0]);
                    tabLayout.getTabAt(1).setIcon(tabsUnselected[1]);
                    tabLayout.getTabAt(2).setIcon(tabsUnselected[2]);
                    tabLayout.getTabAt(3).setIcon(tabSelected[3]);
                    tabLayout.getTabAt(4).setIcon(tabsUnselected[4]);
                    break;


                default:
                    tabLayout.getTabAt(0).setIcon(tabsUnselected[0]);
                    tabLayout.getTabAt(1).setIcon(tabsUnselected[1]);
                    tabLayout.getTabAt(2).setIcon(tabsUnselected[2]);
                    tabLayout.getTabAt(3).setIcon(tabsUnselected[3]);
                    tabLayout.getTabAt(4).setIcon(tabSelected[4]);
                    break;
            }
        }
\$\endgroup\$
  • 1
    \$\begingroup\$ I've edited my post. It's Android SDK (Java) \$\endgroup\$ – r3dm4n Jan 26 '17 at 14:37
  • 1
    \$\begingroup\$ As we all want to make our code more efficient or improve it in one way or another, try to write a title that summarizes what your code does, not what you want to get out of a review. Please see How to get the best value out of Code Review - Asking Questions for guidance on writing good question titles. \$\endgroup\$ – BCdotWEB Jan 26 '17 at 15:17
5
\$\begingroup\$

You check whether the index of the tab equals the position. If this is the case, use the selected icon, otherwise use the unselected one:

for(int i = 0; i < 5; i++) {  // maybe replace 5 with a constant declaration...
    if(i == position)
        tabLayout.getTabAt(i).setIcon(tabSelected[i]);
    else
        tabLayout.getTabAt(i).setIcon(tabsUnselected[i]);
}
\$\endgroup\$
2
\$\begingroup\$

This is how i'd do it:

@Override
public void onPageSelected(int position) {

    int size = 4;
    position = Math.min(position, size);

    for(int i = 0; i <= size; i++){
        if(i == position){
            tabLayout.getTabAt(i).setIcon(tabSelected[i]);
        }
        else {
            tabLayout.getTabAt(i).setIcon(tabsUnselected[i]);
        }
    }

}

Math.min(position,4) ensures this works even if position is bigger 4, as you handeled with the default case. Don't know what more to tell atm, feel free to ask!

I also added a size value which you can set static or dynamic for faster adaption in case size changes. This will keep the value in one place so its easier to maintain.

Edit:

Another approach:

@Override
public void onPageSelected(int position) {

    int size = 4;
    position = Math.min(position, size);

    for(int i = 0; i <= size; i++){
        tabLayout.getTabAt(i).setIcon(tabsUnselected[i]);
    }

    tabLayout.getTabAt(position).setIcon(tabSelected[position]);

}

even less complex but depending on how slow overwriting a Tab is it might really slow down the code, i doubt it will have significant impact.

Edit2:

Florian Salihovic kindly recommends: "size could be set to TabLayout#getTabCount()". You should have a look if this works for you.

\$\endgroup\$
  • 2
    \$\begingroup\$ I was about to post the second version as well. size could be set to TabLayout#getTabCount(). \$\endgroup\$ – Florian Salihovic Jan 26 '17 at 15:14

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.