我们先来介绍写的比较长的函数。
以下代码做了好几件事情。它创建缓冲区、获取页面、搜索继承下来的页面、渲染路径、添加神秘的字符串、生产HTML等等。
// Listing 3-1
public static String testableHtml(
PageData pageData,
boolean includeSuiteSetup
) throws Exception {
WikiPage wikiPage = pageData.getWikiPage();
StringBuffer buffer = new StringBuffer();
if (pageData.hasAttribute("Test")) {
if (includeSuiteSetup) {
WikiPage suiteSetup = PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_SETUP_NAME, wikiPage
);
if (suiteSetup != null) {
WikiPagePath pagePath = suiteSetup.getPageCrawler().getFullPath(suiteSetup);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -setup .")
.append(pagePathName)
.append("\n");
}
}
WikiPage setup = PageCrawlerImpl.getInheritedPage("SetUp", wikiPage);
if (setup != null) {
WikiPagePath setupPath = wikiPage.getPageCrawler().getFullPath(setup);
String setupPathName = PathParser.render(setupPath);
buffer.append("!include -setup .")
.append(setupPathName)
.append("\n");
}
}
buffer.append(pageData.getContent());
if (pageData.hasAttribute("Test")) {
WikiPage teardown = PageCrawlerImpl.getInheritedPage("TearDown", wikiPage);
if (teardown != null) {
WikiPagePath tearDownPath = wikiPage.getPageCrawler().getFullPath(teardown);
String tearDownPathName = PathParser.render(tearDownPath);
buffer.append("!include -teardown .")
.append(tearDownPathName)
.append("\n");
}
if (includeSuiteSetup) {
WikiPage suiteTeardown = PageCrawlerImpl.getInheritedPage(
SuiteResponder.SUITE_TEARDOWN_NAME,
wikiPage
);
if (suiteTeardown != null) {
WikiPagePath pagePath = suiteTeardown.getPageCrawler().getFullPath(suiteTeardown);
String pagePathName = PathParser.render(pagePath);
buffer.append("!include -teardown .")
.append(pagePathName)
.append("\n");
}
}
}
pageData.setContent(buffer.toString());
return pageData.getHtml();
}
上面这个函数的缺点有很多缺点:如代码太长,有太多不同层级的抽象,奇怪的字符串和函数调用,混以双重嵌套、用标识来控制if语句等。
下面我们来介绍怎么去重构上面这种函数的方法。
函数应该做一件事。做好这件事。只做这一件事。
什么叫做函数只做一件事情呢?如果函数只是做该函数名下同一抽象层上的步骤,则函数还是只做了一件事。
要判断函数是否不止做了一件事,还可以看是否能再拆出一个函数,该函数不仅只是单纯地重新诠释其实现。
例如:
//3.3 将设置和拆解包纳到测试页面中
public static String renderPageWithSetupsAndTeardowns(PageData pageData, boolean isSuite) throws Exception{
//1. 判断是否为测试页面
if (isTestPage(pageData)){
//2. 如果是,则容纳进设置和分拆步骤
includeSetupAndTeardownPages(pageData,isSuite);
}
//3. 渲染成HTML
return pageData.getHtml();
}
每个函数就像是文章的一个段落一样,每个段落都描述当前的抽象层级,并引用位于下一抽象层级的后续段落。
例如:
要容纳设置和分拆步骤,就先容纳设置步骤,然后纳入测试页面内容,再纳入分拆步骤。
要容纳设置步骤,如果是套件,就纳入套件设置步骤,然后再纳入普通设置步骤。
要容纳套件设置步骤,先搜索“SuiteSetUp”页面的上级继承关系,在添加一个包括该页面路径的语句。
在搜索…
switch语句的主要问题在于重复。其次违反了单一权责原则,违反了开放闭合原则。
解决方法:利用多态,使用策略模式或状态模式来解决。
函数参数越少越好,最多不能超过3个。
Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius);//将double x和double y重构为 Point类
函数名和函数体所实现的功能要相对应,不要添加多余的额外功能。
例如:
public class UserValidator {
private Cryptographer cryptographer;
public boolean checkPassword(String userName, String password) {
User user = UserGateway.findByName(userName);
if (user != User.NULL) {
String codedPhrase = user.getPhraseEncodedByPassword();
String phrase = cryptographer.decrypt(codedPhrase, password);
if ("Valid Password".equals(phrase)) {
Session.initialize();
return true;
}
}
return false;
}
}
以上代码的副作用在于对Sessio.initialize()的调用。checkPassword函数,顾名思义,是用来检查密码的。名称并未暗示它会初始化该次会话。
不能通过输入参数来输出返回值,容易产生歧义。
例如:
appendFooter(s);//使用输出参数,容易产生迷惑
public void appendFooter(StringBuffer report);
应改为:
report.appendFooter();
一个函数要么做什么事情,要么回答什么事情,但二者不能不能兼得。
例如:
public boolean set(String attribute, String value);
此函数设置某个属性,如果成功返回true,如果不存在那个属性返回false。这样就会导致以下语句:
if(set("username","unclebob"))...
缺点:函数名表意不清楚,函数功能不单一,容易让调用者产生困扰。
解决方案:把指令与询问分隔开来,防止混淆的发生。
if(attributeExists("username"){
setAttribute("username","unclebob");
...
}
使用错误码会导致深层次的嵌套结构。当返回错误码时,就是在要求调用者立即处理错误。
例如:
if(deletePage(page) ==E_OK) {
if (registry.deleteReference(page.name) == E_OK) {
if (configKeys.deleteKey(page.name.makeKey()) == E_OK) {
logger.log("page deleted");
} else {
logger.log("configKey not deleted");
}
} else {
logger.log("deleteReference from registry failed");
}
} else{
logger.log("delete failed");
return E_ERROR;
}
使用异常处理错误码:
try {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
} catch (Exception e) {
logger.log(e.getMessage());
}
try/catch代码块容易搞乱代码结构。
函数应该只做一件事。错误处理就是一件事。如果关键字try在某个函数中存在,它就应该是这个函数的第一个单词,并且在catch/finally代码块后面也不该有其他内容。
所以把try和catch代码块的主体部分抽离出来,形成一个单独的函数。
public void delete(Page page) {
try {
deletePageAndAllReferences(page); }
catch (Exception e) {
logError(e);
}
}
private void deletePageAndAllReferences(Page page) throws Exception {
deletePage(page);
registry.deleteReference(page.name);
configKeys.deleteKey(page.name.makeKey());
}
private void logError(Exception e) {
logger.log(e.getMessage());
}
以上代码将错误处理和真正的删除page函数分离,可以使得代码更加容易理解和修改。
先想什么写什么,然后对代码进行推敲打磨,分解函数、修改名称、消除重复。
以下代码是对本文最开始的过长函数testableHtml的重构。
public class SetupTeardownIncluder {
private PageData pageData;
private boolean isSuite;
private WikiPage testPage;
private StringBuffer newPageContent;
private PageCrawler pageCrawler;
public static String render(PageData pageData) throws Exception {
return render(pageData, false);
}
private static String render(PageData pageData, boolean isSuite) throws Exception {
return new SetupTeardownIncluder(pageData).render(isSuite);
}
private SetupTeardownIncluder(PageData pageData) {
this.pageData = pageData;
testPage = pageData.getWikiPage();
pageCrawler = testPage.getPageCrawler();
newPageContent = new StringBuffer();
}
private String render(boolean isSuite) throws Exception {
this.isSuite = isSuite;
if (isTestPage()) {
includeSetupAndTeardownPages();
}
return pageData.getHtml();
}
private boolean isTestPage() throws Exception {
return pageData.hasAttribute("Test");
}
private void includeSetupAndTeardownPages() throws Exception {
includeSetupPages();
includePageContent();
includeTeardownPages();
updatePageContent();
}
private void includeSetupPages() throws Exception {
if (isSuite) {
includeSuiteSetupPage();
}
includeSetupPage();
}
private void includeSuiteSetupPage() throws Exception {
include(SuiteResponder.SUITE_SETUP_NAME, "-setup");
}
private void includeSetupPage() {
include("SetUp", "-setup");
}
private void includePageContent() {
newPageContent.append(pageData.getContent());
}
private void includeTeardownPages() {
includeTeardownPage();
if (isSuite) {
includeSuiteTeardownPage();
}
}
private void includeTeardownPage() {
include("TearDown", "-teardown");
}
private void includeSuiteTeardownPage() {
include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");
}
private void updatePageContent() {
pageData.setContent(newPageContent.toString());
}
private void include(String pageName, String arg) {
WikiPage inheritedPage = findInheritedPage(pageName);
if (inheritedPage != null) {
String pagePathName = getPathNameForPage(inheritedPage);
buildIncludeDirective(pagePathName, arg);
}
}
private WikiPage findInheritedPage(String pageName) {
return PageCrawlerImpl.getInheritedPage(pageName, testPage);
}
private String getPathNameForPage(WikiPage page) {
WikiPagePath pagePath = pageCrawler.getFullPath(page);
return PathParser.render(pagePath);
}
private void buildIncludeDirective(String pagePathName, String arg) {
newPageContent.
append("\n!include").
append(arg).
append(" .").
append(pagePathName).
append("\n");
}
}
本文源代码地址:github