Wednesday, September 24, 2008

Ant Lab

00. Purpose:

After being introduced to Ant and the other tools available that help check for potential bugs and non-standardized formatting, we apply these new tools toward our previous assignment, CodeRuler, and see how we measure up to the standards we have set in this course. We are to run CheckStyle, FindBugs, and PMD.

I will be focusing on messages mainly on our written code, MyRuler.java, and not too much on IBM’s supplied classes or methods. Their standards are different from the ones we set so it is unfair to judge theirs.

01. Nervous Going In

I have to admit I am a bit nervous going into this assignment still. Unlike the Stack assignment, my code is about to analyzed by a machine and I am anticipating a lot of lines of “use incorrect (xxx)” or “missing that”. I am the farthest thing from the greatest hacker in the world. Hopefully I learn from the mistakes and bad habits and all of these tools (Checkstyle, PMD, FindBugs, etc) will present less findings in the future. Until then, I’ll get my ass handed to me by the machine.

02. Checkstyle Findings

Checkstyle found many of the errors my reviewer, Philip, found in my CodeRuler program. There were about ten errors in all, several of the same type. Below is a list of what checkstyle returned (the bold are those not indicated in my review):

[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:3:8: Unused import - java.util.Iterator.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:5: Using the '.*' form of import should be avoided -
er.*.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:11: Type Javadoc comment is missing an @author tag.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:23:3: Missing a Javadoc comment.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:58:3: Missing a Javadoc comment.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:102: 'else' construct must use '{}'s.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:176: 'if' construct must use '{}'s.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:182: 'if' construct must use '{}'s.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:191: 'if' construct must use '{}'s.
[checkstyle] C:\eclipse\workspace\Game\src\MyRuler.java:193: 'else' construct must use '{}'s.
One of the new errors Checkstyle found was the curly braces {} not being formatted properly or worst yet, non-existent. Another error is the missing javadocs for IBM’s supplied methods. These reported messages points out all the areas we were too lazy to edit and overlooked because those methods were given by IBM. (Note: I should run and hide in shame. )

02a. CheckStyle - If IBM were an ICS 413 student

Holy smokes, IBM’s other classes turned up a lot of messages though. Thank god we aren’t responsible for fixing those… are we? If they were in our class, FAIL! Haha. Well... not likely, but coding standards are very different.

03. PMD Findings…. YIKES!

[echo] PMD found 36 problem(s).

That’s what stared at me when I ran PMD. Luckily I am only responsible for ten of those. A couple interesting points brought up here. It recommended for if-else statements, we should try to present our passing cases first rather than failing cases. It was called ConfusingTernary:
In an "if" expression with an "else" clause, avoid negation in the test. For example, rephrase: if (x != y) diff(); else same(); as: if (x == y) same(); else diff(); Most "if (x != y)" cases without an "else" are often return cases, so consistent use of this rule makes the code easier to read. Also, this resolves trivial ordering problems, such as "does the error case go first?" or "does the common case go first?".
03a. PMD - If IBM were an ICS 413 student

26 problems were not mine. That's all I'm going to say.

04. FindBugs…. Impressive

DB_DUPLICATE_BRANCHES: Method uses the same code for two branches

I knew FindBugs looks for potential code that could cause code to crash. So I was quite shocked to see this. This “bug” seems to focus more on enhancing code rather than potential problems. Anyways, it was useful and I was able to reduce the if statements so there was no duplicate code for the two branches and instead used simply one.

05. Conclusions: Automated vs. Manual Quality Assurance

I think Checkstyle has its limits in some aspects. Philip made some good points where he pointed out areas of redundancy in comments. It was something Checkstyle couldn’t identify because it does not interpret the content of the comments, at least I think so. It only checks if comments exist and if they are in proper format. So I definitely must thank Philip for his efforts for going above and beyond just looking at syntax. That is not saying I am ungrateful for tools such as CheckStyle, PMD, and FindBugs.

FindBugs and PMD are useful for finding really technical issues that perhaps beginner/intermediate programmers are likely to mess up on or not notice. Issues like using the correct modifiers (i.e. final, static) and object types. Concepts that I do not have full grasp of yet, I can rely on FindBugs and PMD to point out for now. I intend to learn more about these as they arise.

Also, I think I have gotten used to the fact that I am bound to code with errors. I am just more concentrated at correcting what Ant presents to me.

No comments: