Let s ask a few questions in response to the action of this chapter:
Paul and I talked about whether this change is a good one. It certainly cleans up the code substantially from the original, removing a lot of duplication. However, to get rid of that duplication, we had to dig a bit deeper into our bag of tricks and use the delegate feature of C#. Arguing for the defense, Paul pointed out that the code is cleaner and that delegates are quite common in C#, so it s not unreasonable to expect people to know how to use them. Arguing for the prosecution , I pointed out that we didn t understand them until we did this, and as we were surely the two smartest programmers in the room at that moment ” 10 P.M. in the back room at Grizzly Peak ”it wasn t necessarily fair to expect others to know even more than we did. And I pointed out that of course we liked the solution because we wrote it.
Our conclusion: narrow call, but it is a righteous change. We find the code to be much shorter, and while there s a little mental bump to get over, adding new behavior will be a matter of rote now, duplicating the style of what s there.
Thinking about it this morning, I believe I ll do one more thing: I ll write a couple of tests testing delegates. The idea is that the test cases for the product should be a place where the programmers look to learn things, so we should write tests about things we re learning. I ll do that later, perhaps even today, and bring you up to date.
Paul said he had a good time and learned some good things, and he wondered how it had been for me. It was very good for me as well. This area had been troubling me for some time. The duplication was there and I couldn t see how to get rid of it. My e-mail advisers had been suggesting delegates for some time, but I foresaw that I was going to have to subclass MenuItem even to get started. (Did you notice that that never happened ? We could perhaps eliminate multiple menu handlers if we did it, but it wasn t necessary to eliminate all the duplication we were worrying about. So much for up-front design.)
It was the process of showing Paul the code and making simple little improvements that helped us find a step that seemed safe enough, and small enough, to start with. Recall that we moved duplication into a method, then factored it out, in just one place, and made that one place work before moving on. After that, everything followed smoothly.
Even so, we went through a number of false starts trying to figure out who was the delegate, where to declare things, and so on. There were a number of points when, had I been alone, I would have given up and backed out my changes. With Paul pairing with me, his energy carried me over those low points, and he always had another idea for something to try. In short, had I tried this on my own, my memory of how it went, and my knowledge of myself , tells me that I would very likely have given up.
Pairing always makes me better, even when I can t get Paul and have to settle for Chet. Seriously: Pairing makes me better.
Well, lots. One thing is something that I realized on the way back to my car. As you can see in the code, we defined the delegate inside the class. We had tried that in a couple of different ways, and until we found this combination, we kept getting compiler errors. The issue ”as you well know if you re ahead of us in the delegate world ”is that a delegate is basically a class, a data type. If you declare that type inside a class, it s really only accessible to that class (apparently even if it is defined public). If you want it accessible elsewhere, it needs to be defined at the class level, not inside a specific class.
It might be interesting to put the delegate setup in the MenuItem by making a separate class. As it is, there will be another tiny method for each menu item that we set up, plus the corresponding named delegate and its initializer. We could probably get all the menu items handled in one menu handler method, which would then dispatch back to the menu item to call the actual processing method. At this moment, I m not sure whether that would be a good idea, but it might be a way to reduce the number of tiny methods. Reducing the number of methods is a priority, but it s the lowest priority on the Simple Code list, so I ll just leave it alone for now until the code calls out for it.
Scanning the code, which is now much easier to do, I see that odd PutText() method with no parameters. Is that some vestige? I ll remove it right now to find out. Visual Studio wasn t very helpful in finding the references to it. Turns out it s used in CustomerTest.cs. I think I ll rename it to CustomerTestPutText() and make it protected. Oops, I meant internal. Protected means only that class and subclasses can access the method. I want other classes in the project to access it ”that s internal. Works fine.
internal void CustomerTestPutText() {
PutText(textbox, model.LinesArray(), model.SelectionStart);
}
Why did I do it that way? Why didn t I just use a comment? Well, because comments are code s way of saying that it doesn t communicate well. So I decided to change the method to express the idea instead of adding a comment. You might well choose otherwise ”your balance is up to you. Why didn t I leave it public? In this application, it doesn t matter, but I felt it expresses better that this method is specialized for one purpose and not intended for general use. I could be wrong.
Now look at the real PutText method. It has that comment about feature envy. Feature envy, according to Martin Fowler, is present when a method seems more interested in another class than in its own. We certainly have that here: the PutText method is talking only to the TextBox. We could fix that rather easily, since we already have our own TextBox class. Should we, or shouldn t we? I d really like to do it, because the change should be so simple. But on the other hand, we were here to do the delegates thing, my pair is long gone, and the code is clearly marked for improvement at some other time. I think I ll ship it as it is.
In the previous chapter ”Chapter 21, Some Things We Ought to Do ”I was considering some pretty high- powered patterns to clean up the situation between the form and the model. Note that sb some very simple refactoring, aimed just at removing duplication, has done most of the job for us. It always seems to turn out that way. Removing duplication is your friend.