HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Setting up radio button click ActionListeners

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
clickradioactionlistenerssettingbutton

Problem

Instead of writing four almost identical setOnClickListener method calls, I opted to iterate over an array, but I'm wondering if this was the best way to do it. Do you have any suggestions for improving it?

/* Set up the radio button click listeners so two categories are not selected
   at the same time. When one of them is clicked it clears the others.
*/
final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};
for (int i = 0; i < 4; i++) {
    final int k = i;
    buttons[i].setOnClickListener(new OnClickListener() {
        @Override
        public void onClick(View v) {
            for (int j = 0; j < 4; j++) {
                if (buttons[j] != buttons[k]) {
                    buttons[j].setChecked(false);
                }
            }
        }
    });
}

Solution

What GUI framework is this? Is the View v being passed to the OnClick method actually the clicked RadioButton? If so, here are some changes to consider

final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};

final OnClickListener onClickHandler =
    new OnClickListener() {
        @Override
        public void onClick(final View v) {
            final RadioButton checkedButton = (RadioButton) v;
            for (final RadioButton button : buttons) {
                if (button != checkedButton) {
                    button.setChecked(false);
                }
            }
        }
    };

for (final RadioButton button : buttons) {
    button.setOnClickListener(onClickHandler);
}


If View v != the checked radio button, then:

final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};
for (final RadioButton thisButton: buttons) {
    thisButton.setOnClickListener(
        new OnClickListener() {
            @Override
            public void onClick(final View v) {
                for (final RadioButton button : buttons) {
                    if (button != thisButton) {
                        button.setChecked(false);
                    }
                }
            }
        }
    );
}

Code Snippets

final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};

final OnClickListener onClickHandler =
    new OnClickListener() {
        @Override
        public void onClick(final View v) {
            final RadioButton checkedButton = (RadioButton) v;
            for (final RadioButton button : buttons) {
                if (button != checkedButton) {
                    button.setChecked(false);
                }
            }
        }
    };

for (final RadioButton button : buttons) {
    button.setOnClickListener(onClickHandler);
}
final RadioButton[] buttons = {radio_books,radio_games,radio_dvds,radio_electronics};
for (final RadioButton thisButton: buttons) {
    thisButton.setOnClickListener(
        new OnClickListener() {
            @Override
            public void onClick(final View v) {
                for (final RadioButton button : buttons) {
                    if (button != thisButton) {
                        button.setChecked(false);
                    }
                }
            }
        }
    );
}

Context

StackExchange Code Review Q#640, answer score: 5

Revisions (0)

No revisions yet.