Summary
In this blog, I describe 'sensing variables', and how you can use them to make large method refactoring easier. I also discuss the potential for tools which could achieve the same effects in a nicer way.
Advertisement
I spend a lot of time looking at ugly code. Sometimes it makes me angry. I see horrible swirls of chaos and it seems that the people who wrote the code just didn’t care. At other times, though, I’m deeply sympathetic. I see deep dark edifices of code with little bright spots, places where it’s obvious that the programmers did care, but they were dealing with something insurmountable, something that they were deeply afraid of, and fear won the day.
I’m not going to get on my hobby horse about writing tests in this blog. If you’ve read my blogs, or heard me talk, you know that I continually point out that work in code covered with tests is qualitatively different. When your code is surrounded with tests, there’s much less fear. Tests are a huge behavioral invariant which gives you feedback when you change your code. They make aggressive refactoring possible... Code can actually get better... Well, enough of the rant. The thing that I'm trying to say is that I believe that this problem, the problem of knowing whether we are changing behavior in unexpected ways when we refactor is a core problem in software engineering.
Forget testing for a second, what we really want to know is whether we are making unintentional behavioral change in our systems when we modify them. Imagine a world in which we see the effects of most changes we make in our systems immediately, where we’re able to decide whether we like them or not and then move on. Test-Driven Development gives us that model of development, but I wonder if we can do more. Maybe we can move to a model where we’re able to “checkpoint” behavior directly in a language or an IDE. In this blog, I’ll talk about how we can do that sort of checkpointing using a manual technique. In a subsequent blog, I’ll talk about how we can move toward tooling that helps us do the same thing more easily.
Sensing Variables
I few years ago, I wrote up a technique that I called using sensing variables. It’s an incredibly simple technique, but it can be very handy.
Here’s an example.
Let’s say that we have a large ugly method and we want to do some refactoring within it:
void process(int id, long location) {
int value;
Dispatcher dispatcher = new Dispatcher(location);
if (...) {
if (...) {
// imagine 500 lines of ugly code here
...
String baseMessage = “@” + getLastMessage();
dispatcher.send(artLevel, 3);
baseMessage += getMaxRate() + “ “ + value + “.optkey”;
// imagine 500 lines of ugly code here
...
}
One of the things that we’d like to do is take all of that work we do to calculate baseMessage and move it into another method but there are some things in the code that get in the way; the call to send() on dispatcher for instance. It occurs right in the middle of our calculation of baseMessage. If we could move that line, dispatcher.send(artLevel, 3), down a few lines, we could have this:
And it would be pretty easy to extract all of the baseMessage calculation into a method named getBaseMessage().
Can we make that change? Well, we can if there aren't any unpleasant side-effects. If send() doesn’t affect the result of getMaxRate() in the next line, then we could move the dispatcher.send(artLevel, 3) statement below the calculation of baseMessage.
How can we tell?
One simple way is to introduce a new variable, a variable that we can use to sense the value of baseMessage and a particular point in the method's execution:
public String calculatedBaseMessage = ””;
void process(int id, long location) {
int value;
Dispatcher dispatcher = new Dispatcher(location);
if (...) {
if (...) {
// imagine 500 lines of ugly code here
...
String baseMessage = “@” + getLastMessage();
dispatcher.send(artLevel, 3);
baseMessage += getMaxRate() + “ “ + value + “.optkey”;
calculatedBaseMessage = baseMessage;
// imagine 500 lines of ugly code here
...
}
Now that we have that variable, we can write tests using it:
void testProcessLocal() {
final int ID = 12;
DispatchCommand command = new DispatchCommand();
command.process(ID, LOCAL_LOCATION);
assertEquals(“”, command.calculatedBaseMessage);
}
void testProcessRemote() {
final int ID = 12;
DispatchCommand command = new DispatchCommand();
command.process(ID, REMOTE_LOCATION);
assertEquals(“”, command.calculatedBaseMessage);
}
Notice that we’re just checking for an empty string in each of these tests. Once we run the tests, we will get failures which will show us the calculated value of baseMessage for particular inputs. Then we can adjust the tests so that they pass. When we do, we’ll have a set of tests that pin down the existing behavior:
void testProcessLocal() {
final int ID = 12;
DispatchCommand command = new DispatchCommand();
command.process(ID, LOCAL_LOCATION);
assertEquals(“@ red 10.3 3.44 .optkey”, command.calculatedBaseMessage);
}
void testProcessRemote() {
final int ID = 12;
DispatchCommand command = new DispatchCommand();
command.process(ID, REMOTE_LOCATION);
assertEquals(“@ red 11.4 4.55 .optkey”, command.calculatedBaseMessage);
}
Now, we can try to move that line of code and see what happens:
public String calculatedBaseMessage = ””;
void process(int id, long location) {
int value;
Dispatcher dispatcher = new Dispatcher(location);
if (...) {
if (...) {
...
String baseMessage = “@” + getLastMessage();
// dispatcher.send(artLevel, 3);
baseMessage += getMaxRate() + “ “ + value + “.optkey”;
calculatedBaseMessage = baseMessage;
dispatcher.send(artLevel, 3); // Moved from above (see commented-out line below declaration of baseMessage)
...
}
When we run the tests, we’ll know whether send() had a side-effect that affected our refactoring. If the tests fail, it did. If they pass, we can have some confidence that the move was safe.
Now, if you’re seeing this for the first time you might be thinking one of two things. One is that you could probably use a debugger for this.. you could set a watch on baseMessage and debug to find out what happens to it. That’s fine as far as it goes, the problem is that debuggers carry us through one path of execution for one set of input values. Sensing variables can be used across many paths of execution with many different input values. We can use them to write several tests against a chunk of code and effectively pin it down from several different directions.
The other thing that you might be thinking about is that it just feels wrong to introduce variables into production code specifically for testing. Frankly, I feel the same way and that’s why I use sensing variables as a temporary technique. I use them to pin down some behavior, then I add code or refactor.. then I delete them.
Often, I can alter the tests that used them so that they are still useful after my refactoring. For instance, if I extracted all that code for baseMessage into a method, I can alter the tests I wrote above so that they look like this:
void testLocalBaseMessage() {
DispatchCommand command = new DispatchCommand();
assertEquals(“@ red 10.3 3.44 .optkey”,
command.getBaseMessage(“red”, “10.3”, “3.44”));
}
void testRemoteBaseMessage() {
DispatchCommand command = new DispatchCommand();
assertEquals(“@ red 11.4 4.55 .optkey”,
command.getBaseMessage(“red”, “11.4”, “4.55”);
}
These tests test essentially the same behavior, but in a narrower sense. They test it directly on the new method we extracted not on the method it came from.
I’ve used sensing variables in a variety of projects. They're useful but they are a little tedious to set up. In my next blog, I’ll show a little tool that you can use to make the use of sensing variables easier, and I'll describe how languages and IDEs could give us direct support for them.
I agree 100% with your suggestion to use sensors to make it possible to test ugly code.
But I have a suggestion. Sometimes, you can't remove the sensors for a long time. So, rather than making them a public field, I suggest making them a public property without a set method. AND make the property name start with something like TEST. So, for example, in the case you demonstrated, I would make the field a property like this:
1) You may not get the chance to remove the sensor for a long time. This naming convention makes it obvious to a person reading this class that this field isn't part of the core functionality, but rather a test sensor.
2) Anyone using the class should not be tempted to access the field/property. Not with a name like that.
Your suggestion is a great one - it reminds me of the old joke "How do you eat an elephant?" Answer: "One bite at a time". If you're faced with fixing/maintaining a large ugly method, sensors can make a daunting task possible. One bite at a time.