Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Loop with switch vs. different loops

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.

like image 312
Cantillon Avatar asked Feb 07 '26 04:02

Cantillon


1 Answers

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".

like image 111
Jon Skeet Avatar answered Feb 08 '26 20:02

Jon Skeet



Donate For Us

If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!