降低 java 方法的圈复杂度答案

【问题标题】：Reducing the cyclomatic complexity of a java method降低 java 方法的圈复杂度
【发布时间】：2015-10-22 15:09:09
【问题描述】：

我有以下方法：

private void setClientAdditionalInfo(Map map, Client client, User user) {

    Map additionalInfo = (Map) map.get("additionalInfo");

    if (checkMapProperty(additionalInfo, "gender")) {
        client.setGender(additionalInfo.get("gender").toString());
    }
    if (checkMapProperty(additionalInfo, "race")) {
        client.setRace(additionalInfo.get("race").toString());
    }
    if (checkMapProperty(additionalInfo, "ethnicity")) {
        client.setEthnicity(additionalInfo.get("ethnicity").toString());
    }
   .....

还有 12 个 if 语句以类似的方式使用。唯一的区别是不同的 setter 方法名称和不同的参数。现在，由于相同的模式一次又一次地重复，有没有办法降低代码复杂度？

【问题讨论】：

您可以创建一个映射{"race": Client::setRace, ...} 或使用反射来为字符串列表找到合适的设置器。不确定这是否会降低复杂性，但可能会减少重复。我想我会保持这种状态。最好想想“为什么要重复 15 次？”而不是想“这段代码到底在做什么？”
一个可能值得问的问题：为什么您的信息会出现在地图中？难道您之前没有将您的信息存储在Client 中吗？通常答案是“不”，您必须硬着头皮，但有时您可以避免完全从地图中填充对象。
你的意思是“圈复杂度”还是“更少的代码行”？对 N 个事物的 for 循环可以增加 2^N 的圈复杂度，即使它可能看起来更好。对于如何改进不改变圈复杂度的代码，您会得到很多答案。
@djechlin 我需要先降低圈复杂度。但如果那不可能，我真的不想至少重复同样的事情。
@djechlin 你能把圈复杂度降低到低于输入的复杂度吗？如果你有 N 个独立属性的集合，这些属性要么存在要么不存在，这会给你2^N 不同的输入。

标签： java if-statement sonarqube cyclomatic-complexity

【解决方案1】：

不容易，也不能不使用反射。

使用反射，您可以遍历值列表并在客户端对象中调用适当的方法。这将摆脱复杂性并变得更清洁/更健壮。但是它也会执行得更慢。

从根本上说，尽管您会一遍又一遍地执行几乎但不完全相同的操作，但这总是很棘手。

【讨论】：

【解决方案2】：

您可以使用 Java 8 函数式接口来做到这一点。它至少会摆脱重复的条件语句。

private void doRepetitiveThing(Map info, String key, Consumer<String> setterFunction) {
   if(checkMapProperty(info, key)) {
       setterFunction.accept(info.get(key).toString());
   }
}

private void setClientAdditionalInfo(Map map, Client client, User user) {

    Map additionalInfo = (Map) map.get("additionalInfo");

    doRepetitiveThing(additionalInfo, "gender", client::setGender);
    doRepetitiveThing(additionalInfo, "race", client::setRace);
    doRepetitiveThing(additionalInfo, "ethnicity", client::setEthnicity);
   .....

【讨论】：

相同的圈复杂度。

【解决方案3】：

不确定这是否真的降低了圈复杂度，但它使代码更漂亮。使用 Java 8 更容易。

private void setClientAdditionalInfo(Map<String, Object> map, Client client, User user) {
    Map<String, ?> additionalInfo = (Map<String, Object>) map.get("additionalInfo");
    setIfPresent(additionalInfo, "gender", client::setGender);
    setIfPresent(additionalInfo, "race", client::setRace);
    setIfPresent(additionalInfo, "ethnicity", client::setEthnicity);
}

private void <T> setIfPresent(Map<String, ?> additionalInfo,
                              String property,
                              Consumer<T> consumer) {
    if (checkMapProperty(additionalInfo, property)) {
        consumer.apply((T)additionalInfo.get(property));
    }
}

如果您愿意，可以拨打Map<String, Consumer<?>> 以避免重复调用。

【讨论】：

【解决方案4】：

假设Client的字段可以设置为null，如果map不包含属性，则可以创建一个类：

class MapWrapper {
   private final Map map;

   MapWrapper(Map map) {
      this.map = map;
   }

   String get(String key) {
       if (checkMapProperty(map,key)) {
          return map.get(key).toString();
       } else {
          return null;
       }
       // or more concisely:
       // return checkMapProperty(map,key) ? map.get(key).toString() : null;
   }
}

然后

private void setClientAdditionalInfo(Map map, Client client, User user) {

    Map additionalInfo = (Map) map.get("additionalInfo");
    MapWrapper wrapper = new MapWrapper(additionalInfo);

    client.setGender(wrapper.get("gender"));
    ...
}

但如果要求在映射中没有对应键的情况下保持Client 的字段不变，这将无济于事。

【讨论】：

【解决方案5】：

我会使用enum 将设置器与地图的键连接起来，使用name().toLowercase() 作为键。

enum Setter {

    Gender {

                @Override
                void set(Client client, Thing value) {
                    client.setGender(value);
                }

            },
    Race {

                @Override
                void set(Client client, Thing value) {
                    client.setRace(value);
                }

            },
    Ethnicity {

                @Override
                void set(Client client, Thing value) {
                    client.setEthnicity(value);
                }

            };

    void setIfPresent(Client client, Map<String, Thing> additionalInfo) {
        // Use the enum name in lowercase as the key.
        String key = this.name().toLowerCase();
        // Should this one be set?
        if (additionalInfo.containsKey(key)) {
            // Yes! Call the setter-specific set method.
            set(client, additionalInfo.get(key));
        }
    }

    // All enums must implement one of these.
    abstract void set(Client client, Thing value);
}

private void setClientAdditionalInfo(Map map, Client client, User user) {
    Map additionalInfo = (Map) map.get("additionalInfo");
    for (Setter setter : Setter.values()) {
        setter.setIfPresent(client, additionalInfo);
    }
}

【讨论】：

相同的圈复杂度。

【解决方案6】：

仅作为回答 OP 问题的练习，我将创建一个 Map 将属性与 setter 接口相关联：

private static Map<String, ClientSetter> setters = new HashMap<>();

interface ClientSetter {
    void set(Client client, Object value);
}

static {
    setters.put("gender", new ClientSetter() {
        @Override
        public void set(Client client, Object value) {
            client.setGender(value.toString());
        }
    });
    setters.put("race", new ClientSetter() {
        @Override
        public void set(Client client, Object value) {
            client.setRace(value.toString());
        }
    });
    setters.put("ethnicity", new ClientSetter() {
        @Override
        public void set(Client client, Object value) {
            client.setEthnicity(value.toString());
        }
    });
    // ... more setters
}

遍历可用属性并为setters 映射中的每个可用属性调用set 方法：

private void setClientAdditionalInfo(Map map, Client client, User user) {

    Map additionalInfo = (Map) map.get("additionalInfo");

    List<String> additionalInfoKeys = new ArrayList<>(additionalInfo.keySet());
    additionalInfoKeys.retainAll(setters.keySet());
    for (String key: additionalInfoKeys) {
        Object value = additionalInfo.get(key);
        setters.get(key).set(client, value);
    }
}

PS：显然这不建议用于生产代码。将集合的所有元素复制到一个列表以使这两个集合相交 - 为了防止正在测试/编写的代码中的条件 - 非常昂贵。

【讨论】：