Какой лучший способ избавиться от вложенных ifs в коде при проверке условий?

Question

Я занимаюсь разработкой приложения BlackBerry на Java, и у меня есть класс Options, в котором хранятся все пользовательские настройки. Проблема в том, что мне нужно проверить некоторые условия, чтобы знать, как реагировать. По мере того, как я продолжаю добавлять больше функций, пользователю показывается больше опций графического интерфейса, больше настроек сохраняется в классе Options, и нужно проверять больше условий.

Возьмите следующий код, например:

private void doCallMonitoring(int callId){
    /*This is the part that I want to avoid. Having
      multiple nested ifs. Here's just two conditions
      but as I add more, it will get unmantainable
      very quickly.*/
    if(Options.isActive().booleanValue()){
        callTime = new Timer();
        TimerTask callTimeTask = new TimerTask(){
            public void run(){
                callTimeSeconds++;
        if((callTimeSeconds == Options.getSoftLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){
                    injectDTMFTone(Phone.getActiveCall());
        }else if((callTimeSeconds >= Options.getHardLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){
                    injectEndCall();
                }
             }
        };     
        callTime.schedule(callTimeTask, 0,1000);
    }else{
    System.out.println("Service not active");
    }
}

Как бы я хотел, чтобы это работало, чтобы проверить все опции одним вызовом и оттуда определить проклятие действия. Как мне добиться такого дизайна?

Answer 1

Другой вариант - заставить такие методы, как injectDMTFTone(), проверять, хотят ли они обработать это условие, и возвращать true или false в зависимости от того, было ли оно обработано или нет.

Например:

public void run() {
    callTimeSeconds++;
    do {
        if (handleInjectDMTFTone())
            break;
        if (handleInjectEndCall())
            break;
    } while(false);

    callTime.schedule(callTimeTask, 0,1000);
}

boolean handleInjectDMTFTone() {
    if ((callTimeSeconds != Options.getSoftLimit().intValue()) ||
        (Phone.getActiveCall().getStatus() != PhoneCall.STATUS_CONNECTED))
        return false;

    injectDTMFTone(Phone.getActiveCall());
    return true;
}

boolean handleInjectEndCall() {

    if ((callTimeSeconds < Options.getHardLimit().intValue()) ||
        (Phone.getActiveCall().getStatus() != PhoneCall.STATUS_CONNECTED))
        return false;

    injectEndCall();
    return true;
}

Конечно, вместо того, чтобы вызывать другой injectDMTFTone() метод или injectEndCall() метод, вы просто вставляете эту логику прямо в эти методы. Таким образом, вы сгруппировали всю логику того, как и когда иметь дело с этими условиями в одном месте.

Это одна из моих любимых моделей; используйте операторы if как можно ближе к началу методов, так как имеет смысл исключать условия и возвращать. Остальная часть метода не имеет отступа на многих уровнях, и ее легко и просто прочитать.

Вы можете дополнительно расширить это, создав объекты, которые все реализуют один и тот же интерфейс и находятся в хранилище обработчиков, которые ваш метод run может перебрать, чтобы увидеть, кто его обработает. Это может или не может быть чрезмерным для вашего случая.

Answer 2

Вы можете использовать рефакторинг «метод извлечения» и объединить все эти проверки в одно «читабельное» условие.

См. Этот связанный ответ Бит длинный, но смысл в замене конструкций следующим образом:

       }else if((callTimeSeconds >= Options.getHardLimit().intValue()) && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)){
                injectEndCall();
            }
         }

Примерно так:

       ....
       }else if(shouldInjectEndCall() ){
                injectEndCall();
            }
         }
       ...

Помните, что объекты имеют состояние и могут использовать другие объекты, чтобы помочь им в работе.

Answer 3

Все эти ответы, вероятно, лучше ОО ответов. На этот раз я пойду за быстрый и грязный ответ.

Мне действительно нравится упростить сложные вложенные ifs, переворачивая их.

if(!Options.isActive().booleanValue()) {
    System.out.println("Service not active");
    return;
}
the rest...

Я знаю, что некоторые люди не любят возвраты среднего метода, но когда вы проверяете условия входа, которые всегда были для меня отличным паттерном, я никогда не жалел об этом.

Это действительно упрощает внешний вид вашего метода.

Если вы пишете методы длиннее экрана, не делайте этого и не пишите большой комментарий, указывающий на это - слишком легко потерять оператор return и забыть, что вы это сделали. А еще лучше, не пишите методы длиннее экрана.

Answer 4

Другой вариант - сделать что-то вроде «заменить условный полиморфизм».

Хотя кажется, что вы просто пишете больше кода, вы можете заменить все эти правила на объект «validator» и иметь все проверки в некотором массиве и проходить через них.

Что-то вроде этого скретч-кода.

  private void doCallMonitoring(int callId){
     // Iterate the valiators and take action if needed. 

      for( Validation validation : validationRules ) { 
          if( validation.succeed() ) { 
              validation.takeAction();
          }
      }
   }

И вы реализуете их так:

abstract class Validation { 

      public boolean suceed();
      public void takeAction();
}

class InjectDTMFToneValidation extends Validation { 
    public boolean suceed() { 
        return (callTimeSeconds == Options.getSoftLimit().intValue()) 
               && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)
     }
     public void takeAction() { 
         injectDTMFTone(Phone.getActiveCall());
     }
}

class InjectEndCallValidation extends Validation { 
    public boolean suceed() { 
        return (callTimeSeconds >= Options.getHardLimit().intValue()) 
                && (Phone.getActiveCall().getStatus() == PhoneCall.STATUS_CONNECTED)
     }
     public void takeAction() { 
         injectEndCall();
     }
}

И, наконец, установите их в списке:

private List<Validation> validationRules = new ArrayList<Validation>();{
   validationrules.add( new InjectDTMFToneValidation() );
   validationrules.add( new InjectEndCallValidation () );
   ...
   ...
}

Идея в том, чтобы переместить логику в подклассы. Конечно, вы получите лучшую структуру и, вероятно, suceed и takeAction могли бы быть заменены другими более значимыми методами, цель состоит в том, чтобы получить подтверждение от того, где оно находится.

Это становится более абстрактным? .. да.

Кстати, почему вы используете классы Options и Phone, вызывая их статические методы вместо использования экземпляров?