... | @@ -32,11 +32,137 @@ One input argument is the next best thing to no arguments. `SetupTeardownInclude |
... | @@ -32,11 +32,137 @@ One input argument is the next best thing to no arguments. `SetupTeardownInclude |
|
|
|
|
|
Output arguments are counterintuitive. Readers expect arguments to be inputs, not out- puts. If your function must change the state of something, have it change the state of the object it is called on. (See "Output Arguments" on page 45. Text reproduced below.)
|
|
Output arguments are counterintuitive. Readers expect arguments to be inputs, not out- puts. If your function must change the state of something, have it change the state of the object it is called on. (See "Output Arguments" on page 45. Text reproduced below.)
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### F3: Flag Arguments
|
|
#### F3: Flag Arguments
|
|
|
|
|
|
Boolean arguments loudly declare that the function does more than one thing. They are
|
|
Boolean arguments loudly declare that the function does more than one thing. They are
|
|
confusing and should be eliminated. (See "Flag Arguments" on page 41. Text reproduced below.)
|
|
confusing and should be eliminated. (See "Flag Arguments" on page 41. Text reproduced below.)
|
|
|
|
|
|
|
|
---
|
|
|
|
|
|
|
|
__Flag Arguments__
|
|
|
|
|
|
|
|
Flag arguments are ugly. Passing a boolean into a function is a truly terrible practice. It immediately complicates the signature of the method, loudly proclaiming that this function does more than one thing. It does one thing if the flag is true and another if the flag is false!
|
|
|
|
|
|
|
|
In Listing 3-7 (code below) we had no choice because the callers were already passing that flag in, and I wanted to limit the scope of refactoring to the function and below. Still, the method call `render(true)` is just plain confusing to a poor reader. Mousing over the call and seeing `render(boolean isSuite)` helps a little, but not that much. We should have split the function into two: `renderForSuite()` and `renderForSingleTest()`.
|
|
|
|
|
|
|
|
```java
|
|
|
|
|
|
|
|
package fitnesse.html;
|
|
|
|
|
|
|
|
import fitnesse.responders.run.SuiteResponder;
|
|
|
|
import fitnesse.wiki.*;
|
|
|
|
|
|
|
|
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);
|
|
|
|
}
|
|
|
|
|
|
|
|
public 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() throws Exception {
|
|
|
|
include("SetUp", "-setup");
|
|
|
|
}
|
|
|
|
|
|
|
|
private void includePageContent() throws Exception {
|
|
|
|
newPageContent.append(pageData.getContent());
|
|
|
|
}
|
|
|
|
|
|
|
|
private void includeTeardownPages() throws Exception {
|
|
|
|
includeTeardownPage();
|
|
|
|
if (isSuite)
|
|
|
|
includeSuiteTeardownPage();
|
|
|
|
}
|
|
|
|
|
|
|
|
private void includeTeardownPage() throws Exception {
|
|
|
|
include("TearDown", "-teardown");
|
|
|
|
}
|
|
|
|
|
|
|
|
private void includeSuiteTeardownPage() throws Exception {
|
|
|
|
include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");
|
|
|
|
}
|
|
|
|
|
|
|
|
private void updatePageContent() throws Exception {
|
|
|
|
pageData.setContent(newPageContent.toString());
|
|
|
|
}
|
|
|
|
|
|
|
|
private void include(String pageName, String arg) throws Exception {
|
|
|
|
WikiPage inheritedPage = findInheritedPage(pageName);
|
|
|
|
if (inheritedPage != null) {
|
|
|
|
String pagePathName = getPathNameForPage(inheritedPage);
|
|
|
|
buildIncludeDirective(pagePathName, arg);
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
private WikiPage findInheritedPage(String pageName) throws Exception {
|
|
|
|
return PageCrawlerImpl.getInheritedPage(pageName, testPage);
|
|
|
|
}
|
|
|
|
|
|
|
|
private String getPathNameForPage(WikiPage page) throws Exception {
|
|
|
|
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");
|
|
|
|
}
|
|
|
|
}
|
|
|
|
|
|
|
|
```
|
|
|
|
|
|
|
|
|
|
|
|
|
|
#### F4: Dead Function
|
|
#### F4: Dead Function
|
|
|
|
|
|
Methods that are never called should be discarded. Keeping dead code around is wasteful. Don't be afraid to delete the function. Remember, your source code control system still remembers it.
|
|
Methods that are never called should be discarded. Keeping dead code around is wasteful. Don't be afraid to delete the function. Remember, your source code control system still remembers it.
|
... | | ... | |