I have a method which loops over user elements, and sets a boolean value according to some given constraint:
public void checkUsers( int constraint ) {
for(int i=0; i<nodeUsers().size(); i++) {
UserElement elem = nodeUsers().getUsersElementAt(i);
switch (constraint) {
case CHECK_ALL:
elem.setChecked(true); break;
case CHECK_NONE:
elem.setChecked(false); break;
case CHECK_NO_LANG:
if (elem.getLanguage() == null)
elem.setChecked(true);
else
elem.setChecked(false);
break;
// More cases
}
}
}
I'm wondering if this solution is OK. Maybe I could better write different methods like:
public void checkAllUsers() {
for(int i=0; i<nodeUsers().size(); i++) {
UserElement elem = nodeUsers().getUsersElementAt(i);
elem.setChecked(true);
}
}
public void checkNoUsers() {
for(int i=0; i<nodeUsers().size(); i++) {
UserElement elem = nodeUsers().getUsersElementAt(i);
elem.setChecked(false);
}
}
// Edit: I've added a third case.
It seems to me that you could do this more effectively by making your constraint a real enum:
public enum Constraint
{
CHECK_NONE
{
@Override void apply(UserElement element)
{
element.setChecked(false);
}
},
CHECK_ALL
{
@Override void apply(UserElement element)
{
element.setChecked(true);
}
};
public abstract void apply(UserElement element);
}
Then you can have:
public void checkUsers(Constraint constraint) {
for(int i=0; i<nodeUsers().size(); i++) {
UserElement elem = nodeUsers().getUsersElementAt(i);
constraint.apply(elem);
}
}
Alternatively, have an interface with the same "apply" method, and pass in an instance of the interface into your checkUsers method. It's the same basic pattern - separate the iteration over all user elements from "what to do with the element".
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With