Wednesday, October 10, 2007

MyIsernReview

Today's assignment is MyIsernReview. I'm reviewing the code of Andrew Wong (team silver). The team silver code is here.


Installation

Downloading, unpacking, verifying, and building the distribution all went fine. When I ran the jarfile, it printed out a nice help message, telling me to use -tabs or -console to choose the type of output. Both ways worked as advertised.

Code Format and Conventions

Per our instructions, I spent only a few minutes looking for violations, and none leapt out at me.

Test Cases

Black Box

The unit tests included these black box tests:

  • Test that the number of records printed is the same as the number of records given.
  • Check that no records are printed when no records exist.
  • Test that the program runs.

The box couldn't be completely black because a lot of the methods being tested print their results. The tests mostly went by status flags in the return values. (For example, the method that prints out records returns the number of records printed and the test checks that return value.)

The problem of testing printed output doesn't have any perfect solution (that I know of) but it might be better to have methods that return the complete output and test that. It would also be good to test the contents of the output and the number of columns.

White Box

Emma reports 100% coverage.

The white box test were geared toward testing the column formatting in console mode. There's a method called getColumnSize that takes an array of strings representing a column and figures out what the width of the column should be. The method actually takes the length of the longest string in the column and uses the next multiple of ten after that, so there are several white box tests using string lengths chosen to test the rounding logic.

There are also tests that the program runs in the right "mode" based on the command-line arguments.

The table structure is basically hard-coded, so a lot of the code is a bunch of method calls that have to be in the right order to put things in the right column. It would be good to test that things are actually going in the right columns.

Breaking

With the XML input given in the specs, there are really only two cases for the program to cover, and it basically goes straight through and does them. The only way I can see to break the program is to drop the assumption in the spec and give it other data, which I think doesn't count.

There is one thing I noticed. It's not technically "incorrect output" but some of the columns in console mode are much wider than they need to be. The data is trimmed of whitespace before it's put in the table, but the getColumnSize method is passed an array of the untrimmed data. That's pretty easy to fix.

Lessons Learned

The main thing I learned is that System.out has a format method that's good for printing columns.

Testing a program like this is a little odd. The input is fixed, so you could just have a test that does a giant string comparison on a string that's printed out as the entire output. In fact, you could have a program that just prints out a hard-coded string.

It's kind of silly, but now that I think of it, I wish I had done that. It would have been a lot easier.

No comments: