我正在阅读一本名为
Clean Code -A Handbook of Agile Software Craftsmanship的书,由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?我究竟做错了什么?你们怎么看待这个?
最佳答案 坦率地说,我认为这个提示是愚蠢的,因为它是如此极端.
就个人而言,如果我要对你的功能做任何事情,我会改变它:
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;
}
好处是:
>您只进行一次地图查找而不是两次.
> list.add()调用在两个地方不重复.
现在,如果你想要考虑一些事情,以下将是一个很好的候选人:
List<Issue> list = map.get(componentName);
if (list == null) {
list = new ArrayList<Issue>();
map.put(componentName, list);
}
如果以上出现在多个地方,我肯定会这样做.否则,可能不是(至少不是最初的).