Cleaning the Code


Now we re finished reflecting, and we re ready to clean up the code. We re going to go over most of the modules, and we begin with ones where we re sure there s something to do.

CustomerTest: ArrayToEnd

CustomerTest has a method ArrayToEnd that we think isn t used any more. We ask Microsoft Visual Studio to give us references to it. It says there are none. We remove the method and compile; the compiler doesn t complain. We run the tests; the tests all run. We re quite sure it was OK to remove ArrayToEnd.

Scaffolding Tests

There are some scaffolding tests left in CustomerTest. Remember that we did a couple of trivial tests on an empty model, and on a constant string, before making the leap to reading a test from a file. We could remove those tests right now, but we don t, for two reasons:

  1. We enter tests in chronological order in the file. Reading down through the tests gives insight into our thinking as we built the object. We think that history is potentially valuable . The tests are harmless, and there s no reason to remove them.

  2. These tests are of very fine grain compared to the customer tests. In a sense, they re like unit tests. If something goes wrong in the innards of CustomerTest, the failure of these tests might well identify the problem more quickly than debugging through a customer test.

We generally think this way: if a test is made a bit redundant by a following test but isn t doing any harm and could perhaps point more sharply to a problem, we tend to leave it in. Probably you could match our behavior to 95 percent by just never deleting a test. We like to reflect on why we do things, to sharpen our own skills and to share our thinking with you.

Rename a Test

One of the early tests is named DirectInput. We had in mind just giving some input to the model and testing it. But in fact, we tested the model without giving it any input, so we decided to rename the test EmptyModel. It looks like this now:

 [Test] public void EmptyModel() { 
model.Enter();
AssertEquals("%lt;P></P>\r\n", model.TestText);
}

We compiled, built, looked at NUnit and marvelled that it had picked up the new name , and ran the tests. Not that we had any doubt ”it s just what we do. Every time. It s fascinating when things work the way they are supposed to.

Rename a Variable

Take a look at the following code:

 [Test] public void TestAllFiles() { 
String[] testFiles = Directory.GetFiles(@"c:\data\csharp\notepad\", "*.test");
foreach (String testFile in testFiles) {
InterpretFileInput(testFile);
}
}

The variable testFile isn t a file; it s the name of a file. We decided to rename it to testFileName. We did that, compiled, ran the tests, and then thought better of the change. We think it s the name of the test file. It s the file name of the test file. So we renamed it testFilename, without the camel cap on name . Maybe we were right to think it was better, maybe we weren t. Your choices will vary, but do spend the moment it takes to give the thing a good name. Don t give it an hour , not even a few minutes, but a moment? Yes.

InterpretFileInput

We have an issue with the InterpretFileInput method and the way it interacts with InterpretCommands. Take a look at it:

 private void InterpretFileInput(String fileName) { 
StreamReader stream = File.OpenText(fileName);
String contents = stream.ReadToEnd();
stream.Close();
InterpretCommands(contents, fileName);
}
private void InterpretCommands(String commands, String message) {
StringReader reader = new StringReader(commands);
String line = reader.ReadLine();
...

Do you see what s happening? InterpretFileInput reads the file into a string, and then InterpretCommands makes a StringReader on that string. Why not just pass a StreamReader on the file into InterpretCommands? That would seem to make more sense.

We decided not to do the change, however. It felt a little larger than the refactorings we were doing, and it would throw off our rhythm to do it. And it s mostly harmless the way it is. Maybe we should have changed it; maybe we ll change it later.

The Read Loops

We have a pattern of code where we read a line from an input stream and then loop while the line isn t null. ReadToEnd, which we already looked at, is an example:

 private String ReadToEnd(StringReader reader) { 
String result = "";
String line = reader.ReadLine();
while (line != null && line != "*end") {
result += line;
result += System.Environment.NewLine;
line = reader.ReadLine();
}
return result;
}

We think there should be a way to do this loop without the duplicated reads, one outside and one inside the loop. Without the check for *end, we could do it with reader.Peek(), which returns -1 if nothing s left to read. (Thanks for saving that feature from C, Microsoft!) But in this case, if we use Peek(), we still have to test for *end and we have to break the loop in that case as well. We typed in the code and it looked worse , so we re living with what we have until we have a better idea.

We re about out of ideas for how to clean up CustomerTest.




Extreme Programming Adventures in C#
Javaв„ў EE 5 Tutorial, The (3rd Edition)
ISBN: 735619492
EAN: 2147483647
Year: 2006
Pages: 291

flylib.com © 2008-2017.
If you may any questions please contact us: flylib@qtcs.net