简化 if else 条件以降低 Java 中的认知复杂性答案

【问题标题】：Simplify if else conditions to reduce cognitive complexity in Java简化 if else 条件以降低 Java 中的认知复杂性
【发布时间】：2020-06-24 11:53:02
【问题描述】：

有没有办法简化这个java函数？

为了代码的可维护性，需要简化。

public void pushDocument(ESDocumentType esDocumentType, Object data, String documentId, long userId, long organizationId) {
    boolean proceed = false;

    if (esDocumentType.equals(ESDocumentType.XMLACTIVITY)) {
        proceed = Constants.ELASTIC_LOGGING_ENABLED && Constants.ELASTIC_XMLACTIVITY_ENABLED || Constants.SQS_LOGGING_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.XMLREQRES)) {
        proceed = Constants.ELASTIC_LOGGING_ENABLED && Constants.ELASTIC_XMLREQRES_ENABLED || Constants.SQS_LOGGING_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.ORDERHISTORY)) {
        proceed = Constants.ELASTIC_LOGGING_ENABLED && Constants.ELASTIC_ORDERHISTORY_ENABLED || Constants.SQS_LOGGING_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.SINGIN)) {
        proceed = Constants.ELASTIC_LOGGING_ENABLED && Constants.ELASTIC_SIGNIN_ENABLED || Constants.SQS_LOGGING_ENABLED;
    } else if (esDocumentType.equals(ESDocumentType.GOOGLESEARCH)) {
        proceed = Constants.ELASTIC_LOGGING_ENABLED && Constants.ELASTIC_GOOGLESEARCH_ENABLED || Constants.SQS_LOGGING_ENABLED;
    }

    if (proceed) {
        LogThread logThread = new LogThread();
        logThread.pushDocument(esDocumentType, data, documentId, userId, organizationId);
    }
}

【问题讨论】：

所有表达式似乎都以Constants.ELASTIC_LOGGING_ENABLED && 开头，以|| Constants.SQS_LOGGING_ENABLED 结尾。把它们拉出来包围现有的条件。
请立即查看
改用开关盒

标签： java sonarqube

【解决方案1】：

我不知道你的确切用例，你必须自己改进一下，但这样的事情可能会奏效。

List<ESDocumentType> enabled; // fill this based on your "Constant.ELASTIC_<BLAH>_ENABLED" constants in the constructor

public void pushDocument(ESDocumentType type, other parameters) {
    boolean proceed = (Constants.ELASTIC_LOGGING_ENABLED && enabled.contains(type)) || Constants.SQS_LOGGING_ENABLED;
    
    if (proceed) {
        LogThread logThread = new LogThread();
        logThread.pushDocument(esDocumentType, data, documentId, userId, organizationId);
    }
}

【讨论】：

优雅，我给你。 +1
我认为这里有一个缺陷。如果Constants.SQS_LOGGING_ENABLED 是true，那么无论是否启用了测试，都将始终进行日志记录。但在原始构造中不会发生这种情况，因为 proceed 只有在匹配其中一种文档类型时才能为真。
@WJS，很公平——这可以通过第二个列表来解决。就像我说的那样，我不知道用例，而且这种差异甚至可能不相关。
如果您刚刚分配了enabled.contains(type) 的输出以继续。如果为真，则表明条件已满足。然后，您可以稍后检查日志记录权限。

【解决方案2】：

使用 switch 语句，我认为它应该像这样工作（未经测试）：

switch(ESDocumentType)
{
   case ESDocumentType.XMLACTIVITY:
        proceed = Constants.ELASTIC_LOGGING_ENABLED && 
        Constants.ELASTIC_XMLACTIVITY_ENABLED || Constants.SQS_LOGGING_ENABLED;
   break;
   
   [ .... add the other cases here]


   default:
     //we do not need to set proceed to false manually, but here would be the case for that
   break;

}

【讨论】：

这是我第一次看到有人建议使用幻数而不是劝阻某人使用它们。
感谢您的建议，但 sonarqube 集成在方法上有一些验证。如果我使用建议的规则，该规则应该会使代码失败。
一个更好的建议是将proceed = Constants.ELASTIC_LOGGING_ENABLED从switch中取出，并在procede &= ...中执行类似procede &= ...的操作
不过，switch 语句并没有真正降低复杂性。
@Taschi 认知复杂性，可以说是“可读性”。

【解决方案3】：

这是一种可能性。如果我这样做，我会设置一个地图来获取适当的布尔值。

public void pushDocument(ESDocumentType esDocumentType,
        Object data, String documentId, long userId,
        long organizationId) {
    
    
    boolean proceed = esDocumentType.equals(ESDocumentType.XMLACTIVITY);
    proceed = proceed || esDocumentType.equals(ESDocumentType.XMLREQRES);
    proceed = proceed || esDocumentType.equals(ESDocumentType.ORDERHISTORY);
    proceed = proceed || esDocumentType.equals(ESDocumentType.SINGIN);
    proceed = proceed ||  esDocumentType.equals(ESDocumentType.GOOGLESEARCH);
    
    
    // if any logging is to be done, proceed and one of the others must be true.
    if (proceed && (Constants.SQS_LOGGING_ENABLED
            || Constants.ELASTIC_LOGGING_ENABLE)) {
            LogThread logThread = new LogThread();
            logThread.pushDocument(esDocumentType, data,
                    documentId, userId, organizationId);
    }
}

这是我提到的替代方案。唯一的区别是proceed 是如何确定的。

Map<ESDocumentType, Boolean> docType = Map.of(
        ESDocumentType.EMLACTIVITY, Constants.ELASTIC_XMLACTIVITY_ENABLED,
        ESDocumentType.XMLREQRES, Constants.ELASTIC_XMLREQRES_ENABLED,
        ESDocumentType.ORDERHISTORY, Constants.ELASTIC_ORDERHISTORY_ENABLED,
        ESDocumentType.SINGIN, Constants.ELASTIC_SINGIN_ENABLED,
        ESDocumentType.GOOGLESEARCH, Constants.ELASTIC_GOOGLESEARCH_ENABLED);
    
public void pushDocument(ESDocumentType esDocumentType,
        Object data, String documentId, long userId,
        long organizationId) {
    
    
    boolean proceed = docType.getOrDefault(esDocumentType, false);
    
    // if any logging is to be done, proceed and one of the others must be true.
    if (proceed && (Constants.SQS_LOGGING_ENABLED
            || Constants.ELASTIC_LOGGING_ENABLE)) { 
                 LogThread logThread = new LogThread();
                 logThread.pushDocument(esDocumentType, data,
                      documentId, userId, organizationId);
    }
}

【讨论】：

它适用于 Java9，我修改了您的代码以支持 Java8

【解决方案4】：

public void pushDocument(ESDocumentType esDocumentType, Object data, String documentId, long userId, long organizationId) {

    boolean proceed = Constants.ELASTIC_LOGGING_ENABLED;

    if (esDocumentType.equals(ESDocumentType.XMLACTIVITY)) {
        proceed &&=  Constants.ELASTIC_XMLACTIVITY_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.XMLREQRES)) {
        proceed &&= Constants.ELASTIC_XMLREQRES_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.ORDERHISTORY)) {
        proceed &&= Constants.ELASTIC_ORDERHISTORY_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.SINGIN)) {
        proceed &&= Constants.ELASTIC_SIGNIN_ENABLED;
    }

    else if (esDocumentType.equals(ESDocumentType.GOOGLESEARCH)) {
        proceed &&= Constants.ELASTIC_GOOGLESEARCH_ENABLED;
    }

    if (proceed || Constants.SQS_LOGGING_ENABLED) {
        LogThread logThread = new LogThread();
        logThread.pushDocument(esDocumentType, data, documentId, userId, organizationId);
    }
}

【讨论】：

【解决方案5】：

对于 Java8 使用下面的代码，这将有助于避免认知和圈复杂度

public void pushDocument(ESDocumentType esDocumentType, Object data, String documentId, long userId, long organizationId) {
    EnumMap<ESDocumentType, Boolean> docType = new EnumMap<>(ESDocumentType.class);
    docType.put(ESDocumentType.XMLACTIVITY, Constants.ELASTIC_XMLACTIVITY_ENABLED);
    docType.put(ESDocumentType.XMLREQRES, Constants.ELASTIC_XMLREQRES_ENABLED);
    docType.put(ESDocumentType.ORDERHISTORY, Constants.ELASTIC_ORDERHISTORY_ENABLED);
    docType.put(ESDocumentType.SINGIN, Constants.ELASTIC_SIGNIN_ENABLED);
    docType.put(ESDocumentType.GOOGLESEARCH, Constants.ELASTIC_GOOGLESEARCH_ENABLED);
    boolean proceed = docType.getOrDefault(esDocumentType, false);
    if (proceed && Constants.ELASTIC_LOGGING_ENABLED || Constants.SQS_LOGGING_ENABLED) {
        LogThread logThread = new LogThread();
        logThread.pushDocument(esDocumentType, data, documentId, userId, organizationId);
    }
}

【讨论】：