#Your actual code
Your actual code
#Your actual code
Your actual code
- Put the
for
loop outside the if-statement continue
early if it is not an instance ofToggleButton
- Adjust the
if
condition to take into consideration thetb.isChecked()
from outside, and use an||
(OR) operation for the logic. - Rename
tb
toclickedButton
which more describes what the button is used for.
ToggleButton tbclickedButton = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) tbclickedButton.getParent();
for (int i = 0; i < gridLayout.getChildCount(); ++i) {
View nextChild = gridLayout.getChildAt(i);
if (!(nextChild instanceof ToggleButton)) {
continue;
}
if (nextChild.getId() != tbclickedButton.getId() || !tbclickedButton.isChecked()) {
ToggleButton tb2 = (ToggleButton) nextChild;
tb2.setChecked(false);
}
}
- Put the
for
loop outside the if-statement continue
early if it is not an instance ofToggleButton
- Adjust the
if
condition to take into consideration thetb.isChecked()
from outside, and use an||
(OR) operation for the logic.
ToggleButton tb = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) tb.getParent();
for (int i = 0; i < gridLayout.getChildCount(); ++i) {
View nextChild = gridLayout.getChildAt(i);
if (!(nextChild instanceof ToggleButton)) {
continue;
}
if (nextChild.getId() != tb.getId() || !tb.isChecked()) {
ToggleButton tb2 = (ToggleButton) nextChild;
tb2.setChecked(false);
}
}
- Put the
for
loop outside the if-statement continue
early if it is not an instance ofToggleButton
- Adjust the
if
condition to take into consideration thetb.isChecked()
from outside, and use an||
(OR) operation for the logic. - Rename
tb
toclickedButton
which more describes what the button is used for.
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);
}
}
#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 ofToggleButton
- Adjust the
if
condition to take into consideration thetb.isChecked()
from outside, and use an||
(OR) operation for the logic.
I believe this code will have the exact same effect as your original:
ToggleButton tb = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) tb.getParent();
for (int i = 0; i < gridLayout.getChildCount(); ++i) {
View nextChild = gridLayout.getChildAt(i);
if (!(nextChild instanceof ToggleButton)) {
continue;
}
if (nextChild.getId() != tb.getId() || !tb.isChecked()) {
ToggleButton tb2 = (ToggleButton) nextChild;
tb2.setChecked(false);
}
}
#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 ofToggleButton
- Adjust the
if
condition to take into consideration thetb.isChecked()
from outside, and use an||
(OR) operation for the logic.
I believe this code will have the exact same effect as your original:
ToggleButton tb = (ToggleButton) view;
RadioGroup gridLayout = (RadioGroup) tb.getParent();
for (int i = 0; i < gridLayout.getChildCount(); ++i) {
View nextChild = gridLayout.getChildAt(i);
if (!(nextChild instanceof ToggleButton)) {
continue;
}
if (nextChild.getId() != tb.getId() || !tb.isChecked()) {
ToggleButton tb2 = (ToggleButton) nextChild;
tb2.setChecked(false);
}
}