Как реорганизовать сложное выражение в условии if?

Question

Вот пример:

if (folderInfoRecord.getValueRequired() 
    && ((!folderInfoData.getInfoType().equals(InfoType.NUMERIC) 
    && !folderInfoData.getInfoType().equals(InfoType.DATE) 
    && !folderInfoData.getInfoType().equals(InfoType.CHOOSE) 
    && StringUtils.isBlank(folderInfoRecord.getInfoValue(), true)) 
    || (folderInfoData.getInfoType().equals(InfoType.NUMERIC) 
    && folderInfoRecord.getInfoValueNumeric() == null) 
    || (folderInfoData.getInfoType().equals(InfoType.DATE) 
    && folderInfoRecord.getInfoValueDateTime() == null) 
    || (folderInfoData.getInfoType().equals(InfoType.CHOOSE) 
    && folderInfoData.getSelectedInfoRole() == null))) { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
} 

Может кто-нибудь сказать мне, как можно оптимизировать это, если условие?

Answer 1

Правильным решением является Replace Conditional with Polymorphism. Самая большая проблема здесь в том, что флаг - это типы кодирования.

---

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

if (folderInfoRecord.getValueRequired() && 
    Checker.isMissing(folderInfoData, folderInfoRecord) 
{ 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
} 

(ниже код может даже не компилировать, пожалуйста, проверьте с вашим оригинал.)

final class Checker { 

    InfoType   it; 
    FolderInfoRecord fir; 
    FolderInfoData fid; 

    private Checker(FolderInfoData fid, FolderInfoRecord fir) { 
    this.it = fid.getInfoType(); 
    this.fid = fid; 
    this.fir = fir; 
    } 

    static public boolean isMissing(FolderInfoData fid, FolderInfoRecord fir) { 

     return (new Checker(fid, fir)).isMissing(); 
    } 

    private boolean isMissing() { 

    return wrongAsNumeric() && wrongAsDate() && wrongAsChoose() && isBlank(); 
    } 

    private boolean wrongAsNumeric() { 

    return it.equals(InfoType.NUMERIC) && (fir.getInfoValueNumeric() == null); 
    } 

    private boolean wrongAsDate() { 

    return it.equals(InfoType.DATE) && (fir.getInfoValueNumeric() == null); 
    } 

    private boolean wrongAsChoose() { 

    return it.equals(InfoType.CHOOSE) && folderInfoData.getSelectedInfoRole() == null) 
    } 

    private boolean isBlank() { 

    return StringUtils.isBlank(folderInfoRecord.getInfoValue(), true); 
    } 
} 

В отличие от других предлагаемых решений, этот код можно использовать повторно: если вы хотите выполнить эту проверку в другом месте кода, вы просто вызываете Checker.isMissing().

Answer 2

Оптимизация, если вы имеете в виду сложность во время выполнения, я не могу сказать, потому что у меня нет туманности, что делает каждый метод. Я бы настоятельно советовал против кода спагетти, как тот, который вы показываете. Для запутанных NOT и ORs я бы избегал иметь .equals или .isBlank вычисляться внутри оператора if. Есть некоторые внешние флаги булевых снаружи, как

bool folderDataisNumeric = folderInfoData.getInfoType().equals(InfoType.NUMERIC) 

, а затем просто использовать флаг! FolderDataisNumeric сделает код более читаемым по-человечески.

Мои два цента, во всяком случае.

Answer 3

final String info = folderInfoData.getInfoType(); 
final boolean isNumeric = info.equals(InfoType.NUMERIC); 
final boolean isDate = info.equals(InfoType.DATE); 
final boolean isChoose = info.equals(InfoType.CHOOSE); 
final boolean isBlank = StringUtils.isBlank(folderInfoRecord.getInfoValue(), true); 

if (folderInfoRecord.getValueRequired() && 
    ((!isNumeric && !isDate && !isChoose && isBlank) 
    || (isNumeric && folderInfoRecord.getInfoValueNumeric() == null) 
    || (isDate && folderInfoRecord.getInfoValueDateTime() == null) 
    || (isChoose && folderInfoData.getSelectedInfoRole() == null))) { 

    showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
    return; 
} 

Answer 4

Вы также можете разделить логику if, чтобы сделать ее более читаемой. В качестве альтернативы я предлагаю вам создать функцию, которая будет выполнять все эти проверки и вернуть true для false. Таким образом, вам не нужно ставить все чеки в одном месте. Вы не получите каких-либо преимуществ в производительности, выполнив предложенные мной подходы, но это будет более читаемо.

Approach1:

infoType = folderInfoData.getInfoType(); 

if (folderInfoRecord.getValueRequired()) 
{ 
    if ((!infoType.equals(InfoType.NUMERIC) && !infoType.equals(InfoType.DATE) && !infoType.equals(InfoType.CHOOSE) && StringUtils.isBlank(folderInfoRecord.getInfoValue(), true))) 
    { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
    } 
    if((infoType.equals(InfoType.NUMERIC) && folderInfoRecord.getInfoValueNumeric() == null)) 
    { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
    } 
    if(infoType.equals(InfoType.DATE) && folderInfoRecord.getInfoValueDateTime() == null) 
    { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
    } 
    if(infoType.equals(InfoType.CHOOSE) && folderInfoData.getSelectedInfoRole() == null) 
    { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
    } 
} 

подход 2:

if(checkConditions(folderInfoData,folderInfoRecord) == true) 
{ 
    showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
    return; 
} 


public bool checkConditions(Object folderInfoData, Object folderInfoRecord) 
{ 
    infoType = folderInfoData.getInfoType(); 

    if (folderInfoRecord.getValueRequired()) 
    { 
     if ((!infoType.equals(InfoType.NUMERIC) && !infoType.equals(InfoType.DATE) && !infoType.equals(InfoType.CHOOSE) && StringUtils.isBlank(folderInfoRecord.getInfoValue(), true))) 
     { 
      return true; 
     } 
     if((infoType.equals(InfoType.NUMERIC) && folderInfoRecord.getInfoValueNumeric() == null)) 
     { 
      return true; 
     } 
     if(infoType.equals(InfoType.DATE) && folderInfoRecord.getInfoValueDateTime() == null) 
     { 
      return true; 
     } 
     if(infoType.equals(InfoType.CHOOSE) && folderInfoData.getSelectedInfoRole() == null) 
     { 
      return true; 
     } 
    } 
    return false; 
} 

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

Answer 5

Ну, если я читаю это право, это сократило бы до 4 вызовов функций, чтобы определить, нужно ли поднимать предупреждение. Так что 1 меньше, по крайней мере ... немного быстрее.

О, да, также читается.

UPDATE:

folderInfoData.getInfoType() не должен быть оценен для всех 3-х типов безоговорочно, измененных, чтобы отразить это. Все еще читаемо.

if(folderInfoRecord.getValueRequired()) 
{ 
    Boolean alertNeeded = false; 

    if(folderInfoData.getInfoType().equals(InfoType.NUMERIC)) 
    { 
     if(folderInfoRecord.getInfoValueNumeric() == null) alertNeeded = true; 
    } 
    else if(folderInfoData.getInfoType().equals(InfoType.DATE)) 
    { 
     if(folderInfoRecord.getInfoValueDateTime() == null) alertNeeded = true; 
    } 
    else if(folderInfoData.getInfoType().equals(InfoType.CHOOSE)) 
    { 
     if(folderInfoData.getSelectedInfoRole() == null) alertNeeded = true; 
    } 
    else 
    { 
     if(StringUtils.isBlank(folderInfoRecord.getInfoValue(), true)) 
     alertNeeded = true; 
    } 

    if(alertNeeded) 
    { 
     showNotification(pageResourceBundle.getText("JS_ALERT_FIELD_REQUIRED")); 
     return; 
    } 
}