Python中文网

一个关于 编程问题的解答网站.

有 Java 编程相关的问题?

你可以在下面搜索框中键入要查询的问题!

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

我正在读一本名叫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?我做错了什么?你们怎么看


共 (2) 个答案

  1. # 1 楼答案

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

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

    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);
            }
    

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

  2. # 2 楼答案

    我认为简化条件本身更有意义。而不是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;
    }