Commenting Means Not Done


Kent Beck taught me that if the code needs a comment, we should take that as a sign that the code isn t done yet: either it doesn t work, or it isn t good enough. Let s see whether we can get rid of some of those comments, making the code so good that it doesn t need them.

Easy Pickins

Let s start with something easy, that first comment: Handle the Enter Key . We re trying to express the idea that the method named Enter handles the processing of the Enter key. I like the word process better than handle. So I think maybe I should remove that first comment, and rename the method from Enter to ProcessEnterKey. But wait, the next comment points out that what we do on Enter is insert a blank line and a P tag. Maybe we should deal with that instead. The code that calls Enter, in the KeyDownHandler, looks like this:

 if (kea.KeyCode == Keys.Enter) { 
model.Enter();
kea.Handled = true;

I m torn between two ways to handle this issue. One is to rename the Enter method to ProcessEnterKey, but that will leave the next concern to be expressing insertion of the P tag. And we need to decide who should know that Enter means to do that. I m inclined to think that the TextModel doesn t know why it is doing this and that we should rename this method for what it does, not why or when it does it. So I m going to rename the method to InsertParagraphTag, at least for now. So the KeyDownHandler code becomes

 if (kea.KeyCode == Keys.Enter) { 
model.InsertParagraphTag();
kea.Handled = true;

And the Enter method looks like this:

 public void InsertParagraphTag() { 
//
// On Enter, we change the TextModel lines to insert, after the line
// containing the cursor, a blank line, and a line with < P >< /P > . We set the new
// cursor location to be between the P tags: < P > < /P > .
//
// calculate which line has the cursor
int cursorLine = CursorLine();
...
}

My first inclination was to remove that whole first comment because I expect to express all those facts by the time I m done. But I m looking ahead, based on a lot of experience. Let s just see how much of the comment we can legitimately remove. I ve already removed the part about processing the Enter key. That fact is expressed in the code from KeyDownHandler, which pretty clearly says that if the key code is enter, we should InsertParagraphTag. I feel that a comment saying that wouldn t help anyone . In the InsertParagraphTag method itself, can we remove some or all of the long comment? What if we named the method like this?

 InsertBlankLineAndParagraphTagAndSetCursorBetweenParagraphTag 

Frankly, I don t think that would help.

I feel that the essence of this method is insertion of the paragraph tag and that the details don t need to be in the name . We might decide not to include the blank line, for example. So let s wait and see.

Well, the easy pickin s turned out to be easy enough, but you see the thinking that went into just that simple Rename Method refactoring. We had a couple of approaches we could have taken, and we re still not entirely satisfied we ve got it good enough. Let s reflect a moment and then move on.

Lesson  

Names trump comments. Faced with a method name that communicates intent or a cryptic name with a comment, I prefer the intention - revealing name every time. You should, too. It will help those who have to read your code later. And remember: the most likely those who will have to read your code later is you!

The Cursor Line

Well, the next line and comment are about calculating which line has the cursor. I like the variable name OK, cursorLine, but I m not so happy with the method name, CursorLine. How about we call that method FindLineContainingCursor or LineContainingCursor or something like that? I think I ll call it LineContainingCursor. I ll change the line that sends the message and the line that defines the method. So now the line has no comment, and it reads

 int cursorLine = LineContainingCursor(); 

Things are a bit better now. I ll show the code as it now looks, and then I m going to bed. Important point here:

Lesson  

Release early and often. I know I m not done refactoring this code, but on a real project, rather than hold it out of the code base overnight, I could, and should, release it. The code is better than it was, and it s running all the tests. I might wish to hold it out, but if I were working in a team environment, it would be better to keep the code in the repository in case someone else needed to work on it or understand it. I believe the best approach is to keep the best possible code, and the most current code, in the repository at all times.

Anyway, here s where we are:

 public void InsertParagraphTag() { 
//
// On Enter, we change the TextModel lines to insert, after the line
// containing the cursor, a blank line, and a line with < P >< /P > . We set the
// new cursor location to be between the P tags: < P > < /P > .
//
int cursorLine = LineContainingCursor();
// allocate a new line array with two more lines
String[] newlines = new String[lines.Length+2];
// copy line 0 through cursor line to output
for(int i = 0; i <= cursorLine; i++) {
newlines[i] = lines[i];
}
// insert blank line
newlines[cursorLine+1] = "";
// insert P tag line
newlines[cursorLine+2] = "<P></P>";
// copy lines after cursor lien to output
for (int i = cursorLine+1; i < lines.Length; i++) {
newlines[i+2] = lines[i];
}
// save new lines as result
lines = newlines;
// set cursor location
selectionStart = NewSelectionStart(cursorLine + 2);
}

Here s what I ll be thinking about for next time. The next few patches of code are dealing with creating a new line array and then filling it in. I ll want to be using Extract Method to make them more meaningful, with names like CopyThroughCursor, InsertPTag, CopyAfterCursor, or something like that. I note that to do that, I probably won t want newlines to be a local variable because the individual methods will each want to process the newlines array. I can make it a member variable easily enough, but I don t feel entirely comfortable with that: a guideline that I use is that all the variables in an object should change at the same frequency, and it feels to me that lines changes at the beginning and end, and newlines changes a lot, in the middle.

Also, C# includes a class called ArrayList, which is like an array except that you can insert into it and such. I m tempted to change TextModel to use ArrayList. But I would have to know more about ArrayList before I can decide on that.

start sidebar
Lesson: Digression ”Doing It Right

At this point, some readers are probably saying something like, If you had spent more time thinking about this at the beginning, you wouldn t have to be doing all this changing now. And it s true: if we had thought a long time at the beginning, and if we were very smart, I might not be doing this changing now. We might also not have any running code at all. Chet and I are very comfortable working this way, through lots of practice. I like it that I can walk away tonight and not address this problem again until I m home two days from now, maybe with Chet s help, and that the code is working fine now, so that what I m working on is cosmetic, not functional. But you might find comfort working with more thought up front.

I d urge you, though, to push the refactoring approach all the way to the limits, like we re doing here, at least for a while. That way, you ll know what it really feels like rather than be guessing. You may be feeling that I m doing a lot of rework , but that s not what s really happening. Chet and I didn t do much thinking up front about what to call these methods and lines of code ”we deferred that thinking almost entirely, and we re doing it now. We agree that time must be spent thinking about these matters. We re just saying that we can defer that thinking until later. Later (that is, now), we know more about what the code really wants to be, so we re better qualified to give things good names and arrange things for best effect.

If we were to stop with the code as originally written, that could be bad: it s not very clear and it might be hard to maintain. If we were to think more up front, we might make better decisions, but they would be made in the absence of the learning we gained by making this code work.

There s an old saying in programming: Make it work, make it right, make it fast. That s what s happening here. We have made it work. Now we re in the process of making it right, and if we need to, later, we ll make it fast. There s nothing new under the sun: the XP programming style is Make it work, make it right, make it fast. We just know a bit more about how to go about it.

You think on that. I m going to bed now, and I ll work on this more when I get back to Michigan.

Lesson  

Ron of the future here again. Upon waking up (literally and figuratively), I realized that I had gotten lucky. My tests weren t really there to support my refactoring. Here s what I wrote next:

end sidebar
 



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