1. Installation Review:
Installation is smooth. I did not encounter any problem when installing and trying to use this package.
Junit, PMD, Checkstyle, Findbugs are all passed with no errors or warnings.
Emma Coverage summary
class: 100% (1/1)
method: 100% (9/9)block: 93% (726/784)
line: 92% (140.1/152)
This is a good coverage.
ant -f dist.build.xml also BUILD SUCCESSFUL.
However, there is a small problem that when running java -jar webspider-chiaofen.jar-totallinks http://hackystat.org 100, It printed out plenty of empty lines. It may because of the logging scheme, which was implemented with println().
2. Code format and conventions review
|WebSpiderExample.java||25-31||EJS-39||Document all members.|
|WebSpiderExample.java||294||EJS-32||Document is unrelative to the method, which is copy-paste from another method.|
|WebSpiderExample.java||241||EJS-33||Keep comments and code in sync.|
|WebSpiderExample.java||299||EJS-9||Use meaningful names.|
3. Test case review
- Black box perspective.
getNumLinks(), this method is tested properly.
isNum(String s), only method is called. Result is not verified properly.
logList (String urlLink, int numLink, String logging), same as above. But this is a method returns void, verify result may be difficult. However, I think do some digging into the data of the test object is able to verify as least a bit of the result, while author verify none.
- White box perspective.
- Break da buggah.
When using -totallinks http://www.google.com 99999999999999999, it throw out a java.lang.NumberFormatException and break down.
4. Summary and Lessons Learned
We should really do out test right, use it to test our code, not to pass the QA tools.
My code does not do well when I test with the arguments above which cause problem. I didn't think of these things either. I only catch them and give out a warning. I think I can do it better by adding more useful statement that will help user use it right.