Toggling between radio buttons

I have several buttons and I want to toggle between them like radio buttons.

The way I have it set up now is that each button have this same code but the buttonValue varies depending on which button they click on.

ToggleButton one;
one = (ToggleButton) view.findViewById(R.id.number_one);
one.setOnClickListener(new View.OnClickListener()  {
    public void onClick(View view) {
        buttonValue = 1;
        ToggleButton tb = (ToggleButton) view;
        if(tb.isChecked()) {
            RadioGroup gridLayout = (RadioGroup) tb.getParent();
            for(int i=0; i<(gridLayout).getChildCount(); ++i) {
                View nextChild = (gridLayout).getChildAt(i);
                if(nextChild instanceof ToggleButton && nextChild.getId()==tb.getId()){

                }else if (nextChild instanceof ToggleButton && nextChild.getId()!=tb.getId() ){
                    ToggleButton tb2=(ToggleButton) nextChild;
                    tb2.setChecked(false);
                }
            }
        } else{
            RadioGroup gridLayout = (RadioGroup) tb.getParent();
            for(int i=0; i<(gridLayout).getChildCount(); ++i) {
                View nextChild = (gridLayout).getChildAt(i);
                if(nextChild instanceof ToggleButton && nextChild.getId()==tb.getId() ){
                    ToggleButton tb2=(ToggleButton) nextChild;
                    tb2.setChecked(false);
                }else if (nextChild instanceof ToggleButton && nextChild.getId()!=tb.getId() ){
                    ToggleButton tb2=(ToggleButton) nextChild;
                    tb2.setChecked(false);
                }
            }
        }
    }
});

I basically have the same code for 10 buttons! While the code works and the buttons toggle between each other, I don't think is the best approach (especially because it's so many lines of code for 10 buttons where the main isChecked() function is exactly the same in every onClick button listener.

I was wondering if there was a way

The trick here is to create an inner class.

Something I've started doing recently is to create a method that returns the interface you want!

private View.OnClickListener createClickListener(final int value) {
    return new View.OnClickListener()  {
        buttonValue = value;
        // the rest of your code goes here
    };
}

Thanks to marking the int value as final in the method declaration, it is accessible also from the inner class.

You can now use this like:

one.setOnClickListener(createClickListener(1));

Your actual code

While reducing the code duplication for the individual buttons is important, let's take a look at the code that is actually performed for each button!

Step 1. Adjust spacing to adhere to the Java coding standards and remove unnecessary parenthesis.

Step 2. gridLayout is used in both if and else, so put it outside.

Step 3. In the else part, you're doing the same thing no matter if nextChild.getId() and tb.getId() is the same or not, so reduce code duplication by simplifying that logic.

Step 4. Remove empty if statement

After these steps, the code is:

ToggleButton tb = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) tb.getParent();
if (tb.isChecked()) {
    for (int i = 0; i < gridLayout.getChildCount(); ++i) {
        View nextChild = (gridLayout).getChildAt(i);
        if (nextChild instanceof ToggleButton && nextChild.getId() != tb.getId()) {
            ToggleButton tb2 = (ToggleButton) nextChild;
            tb2.setChecked(false);
        }
    }
} else {
    for (int i = 0; i < gridLayout.getChildCount(); ++i) {
        View nextChild = gridLayout.getChildAt(i);
        if (nextChild instanceof ToggleButton) {
            ToggleButton tb2 = (ToggleButton) nextChild;
            tb2.setChecked(false);
        }
    }
}

Next steps:

• Put the for loop outside the if-statement

• continue early if it is not an instance of ToggleButton

• Adjust the if condition to take into consideration the tb.isChecked() from outside, and use an || (OR) operation for the logic.

• Rename tb to clickedButton which more describes what the button is used for.

I believe this code will have the exact same effect as your original:

ToggleButton clickedButton = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) clickedButton.getParent();
for (int i = 0; i < gridLayout.getChildCount(); ++i) {
    View nextChild = gridLayout.getChildAt(i);
    if (!(nextChild instanceof ToggleButton)) {
        continue;
    }
    if (nextChild.getId() != clickedButton.getId() || !clickedButton.isChecked()) {
        ToggleButton tb2 = (ToggleButton) nextChild;
        tb2.setChecked(false);
    }
}

StackExchange Code Review Q#67852, answer score: 8