java foreach语句中的块是否必须只有一行？

Question

我正在读一本名叫Clean Code -A Handbook of Agile Software Craftsmanship的书，作者是罗伯特·C·马丁（Robert C.Martin），他在书中给出了很多有用的技巧，告诉我们如何编写好的Java代码

其中一个建议是：

Blocks within if statements, else statements, for statements, and so on should be one line long. Probably that line should be a function call. Not only does this keep the enclosing function small, but it also adds documentary value because the function called within the block can have a nicely descriptive name

对我来说，这是一个非常奇怪的提示，因为从下面的代码：

public Map<String, List<Issue>> mapComponentToIssueList(List<Issue> issues) {
    Map<String, List<Issue>> map = new HashMap<String, List<Issue>>();

    for (Issue issue : issues) {
        String componentName = issue.getComponents().iterator().next().getString("name");
        if (map.containsKey(componentName)) {
            map.get(componentName).add(issue);
        } else {
            List<Issue> list = new ArrayList<Issue>();
            list.add(issue);
            map.put(componentName, list);
        }
    }
    return map;

}

利用这个原理，我得到了：

public Map<String, List<Issue>> mapComponentToIssueList(List<Issue> issues) {
    Map<String, List<Issue>> componentNameIssueListMap = new HashMap<String, List<Issue>>();
    for (Issue issue : issues) {
        populateMapWithComponenNamesAndIssueLists(componentNameIssueListMap, issue);
    }
    return componentNameIssueListMap;
}

private void populateMapWithComponenNamesAndIssueLists(Map<String, List<Issue>> componentNameIssueListMap, Issue issue) {
    String componentName = getFirstComponentName(issue);
    if (componentNameIssueListMap.containsKey(componentName)) {
        componentNameIssueListMap.get(componentName).add(issue);
    } else {
        putIssueListWithNewKeyToMap(componentNameIssueListMap, issue, componentName);
    }
}

private void putIssueListWithNewKeyToMap(Map<String, List<Issue>> componentNameIssueListMap, Issue issue, String componentName) {
    List<Issue> list = new ArrayList<Issue>();
    list.add(issue);
    componentNameIssueListMap.put(componentName, list);
}

private String getFirstComponentName(Issue issue) {
    return issue.getComponents().iterator().next().getString("name");
}

因此，基本上，代码的大小增加了一倍。有用吗？-也许吧

在我的示例中，什么代码被称为clean？我做错了什么？你们怎么看

Answer 1

我认为简化条件本身更有意义。而不是if块的内容，即

public void method(){
...
  if( mycondition1 && mycondition2 && mycondition3 && mycondition4 && mycondition5 && mycondition6 && mycondition7 && mycondition8 ) {
   dosomething();
  }
...
}

变成

public void method(){
...
  if( conditionsAreTrue() ) {
   dosomething();
  }
...
}

boolean conditionsAreTrue(){
return  mycondition1 && mycondition2 && mycondition3 && mycondition4 && mycondition5 && mycondition6 && mycondition7 && mycondition8;
}

Answer 2

坦率地说，我认为这个建议很愚蠢，因为它太极端了

就我个人而言，如果我要对你的功能做任何改变，我会这样做：

public Map<String, List<Issue>> mapComponentToIssueList(List<Issue> issues) {
    Map<String, List<Issue>> map = new HashMap<String, List<Issue>>();

    for (Issue issue : issues) {
        String componentName = issue.getComponents().iterator().next().getString("name");
        List<Issue> list = map.get(componentName);
        if (list == null) {
            list = new ArrayList<Issue>();
            map.put(componentName, list);
        }
        list.add(issue);
    }
    return map;
}

好处是：

1. 只需执行一次地图查找，而不是两次
2. list.add()调用不会在两个地方重复

现在，如果你想解决一些问题，下面是一个很好的选择：

        List<Issue> list = map.get(componentName);
        if (list == null) {
            list = new ArrayList<Issue>();
            map.put(componentName, list);
        }

如果以上内容出现在多个地方，我肯定会这么做。否则，可能不会（至少最初不会）