петли с переключателем против различных петель

Question

У меня есть метод, который перебирает пользовательские элементы, и устанавливает логическое значение, по некоторому данному ограничению:

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   
     } 
    } 
} 

Я интересно, если это решение ОК. Может быть, мне лучше написать разные методы:

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: Я добавил третий случай.

Answer 1

Мне кажется, что вы могли бы более эффективно делать это, делая ваше ограничение реального перечисления:

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); 
} 

Тогда вы можете иметь:

public void checkUsers(Constraint constraint) { 
    for(int i=0; i<nodeUsers().size(); i++) { 
     UserElement elem = nodeUsers().getUsersElementAt(i); 
     constraint.apply(elem); 
    } 
} 

в качестве альтернативы, есть интерфейс Wi один и тот же метод «применить» и передать экземпляр интерфейса в ваш метод checkUsers. Это тот же базовый шаблон - отделяйте итерацию от всех пользовательских элементов от «что делать с элементом».

Answer 2

Реализация двух методов лучше. Ваш код будет легче понять другим, поскольку им нужно будет только прочитать имена методов, а не понимать, что ваш параметр делает в первой реализации.

Answer 3

я не понимаю, почему нет, до тех пор, как вы документировать все правильно :)

можно вычислить длину вашего цикла заранее (экономит время & ресурсы)
так

. nodeUsers() размер() -> сделать его переменной :)

удачи :)

Answer 4

Второй будет работать немного лучше, но он будет меняться в зависимости от настроек JIT.

В худшем случае ограничение будет проверяться на каждой итерации, что может привести к плохой производительности. В лучшем случае ограничение проверяется только один раз.

Альтернативное решение, которое имеет все преимущества, и ни один из потенциальных недостатков:

public void checkUsers(bool setTo) { 
    for(int i=0; i<nodeUsers().size(); i++) { 
     UserElement elem = nodeUsers().getUsersElementAt(i); 
     elem.setChecked(setTo); break; 
    } 

Answer 5

Учитывая, что вы застряли в 1.4, я предлагаю извлечь из цикла инструкцию case, так что, хотя вы не избегаете условного, вы, по крайней мере, изолируете ее. Цикл становится намного проще, циклическая сложность снижается, и когда вы переходите на более современную Java, у вас меньше кода для поиска конверсий (ну, в одной функции вы будете модернизировать, чтобы использовать foreach; в другой, использовать более перечисления - но это разделение интересов):

public void checkUsers(int constraint) { 
    for (int i = 0; i < nodeUsers().size(); i++) 
     checkUser(constraint, nodeUsers().getUsersElementAt(i)); 
} 

private void checkUser(int constraint, UserElement elem) { 
    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 
    } 
} 

Это не зависит от расколоть ли отдельные случаи вверх в их собственные функции.

Answer 6

Это обычный вариант на шаблоне посетителя. Отсутствие поддержки дженериков - это не проблема, так как вы отправляете только один тип значения.Реализация интерфейса (или абстрактного типа) представляется наиболее правильной; это позволяет упростить модульное тестирование и открывает дверь для лучшей расширяемости, если ваш случай развертывания делает инъекцию зависимостей привлекательной.

Answer 7

О, черт возьми. Это лучше:

private void checkUser(int constraint, UserElement elem) { 
    elem.setChecked(shouldCheck(constraint, elem)); 
} 

private boolean shouldCheck(int constraint, UserElement elem) { 
    switch (constraint) { 
    case CHECK_ALL: return true; 
    case CHECK_NONE: return false; 
    case CHECK_NO_LANG: return elem.getLanguage() == null; 
    // More cases 
    } 
} 

Вы избегаете всех разрыва с.

Answer 8

Вы упомянули, что вы застряли в 1.4 в комментарии, так что это должно заставить вас начать с чего-то похожего на то, что предложил Джон Скит, но как типичное перечисление.

public abstract class Constraint { 
    public static final Constraint CHECK_NONE = new Constraint(){ 
     @Override 
     public void apply(UserElement elem) { 
     elem.setChecked(false); 
     } 
    }; 
    // etc. for other cases... 

    // reduce visibility of ctor 
    private Constraint() {}; 

    public abstract void apply(); 
} 

Использование такое же, как в ответе Джона Скита.

Обратите внимание, что это необходимо, только если вы застряли на 1.4. Если вы используете современную Java, используйте реальное перечисление.