Refactoring in the Wild

The past few days, I've been reviewing some code that looks like it could use some love.

It's part of an adapter, that connect the core logic running in our process with some shared mutable state which is managed by Postgres.


It's code that works, rather than clean code that works.  The abstractions aren't very good, separable concerns are coupled to no great advantage, division of work is arbitrary.  And yet...

In the course of our recent work, one of the developers noticed that we could eliminate a number of calls to the remote database by making a smarter version of the main query.  We're using JDBC, and in this case the change required modifying the select clause of the query and making the matching change in the handling of the result set.

Both bits of code were in the same function and fit on the same screen.  There's a duplicated pattern for working with queries, connections, statements, recordsets -- but nobody had come along and tried to "eliminate" that duplication, so the change was easy.

Also, because of changes we've made to the hosting of the database, we needed to change the strategy we use for managing the connection to the database.  That change ended up touching a lot of different methods that were accessing the connection data member directly - so we needed to do an Encapsulate Variable refactoring to avoid duplicating the details of the strategy everywhere.


Applying that refactoring today was no more difficult than introducing it six years ago would have been.


YAGNI... yet.

QotD: Kata Kata Kata!

If you don't force a kata to yield insights about working on real problems, you are wasting opportunities.

A Lesson of a Small Refactoring

We demand rigidly defined areas of doubt and uncertainty. -- Douglas Adams

Working through a Fibonacci number kata last night, I discovered that I was frequently using a particular pattern in my refactoring.


What this gives me in the code is clear separation between the use cases that are not currently covered by tests. I can then be more aggressive in the section of code that is covered by tests, because I've already mitigated the risk of introducing an inadvertent change.

The early exit variation I didn't discover until later, but I think I like it better.  In particular, when you get to a state where the bottom is done, the branch with the early exit gets excised and everything just falls through to the "real" implementation.

This same trick shows up again when it is time to make the next change easy:

It's a bit odd, in that we go from an implementation with full line coverage to one with additional logic that reduces the line coverage. I'm OK with this; it is a form of scaffolding that isn't expected to survive until the implementation is published.

What I find intriguing here is the handling of the code path that doesn't yet have tests, and the inferences one might draw for working on "legacy" code....

Why I Practice TDD

TL; DR: I practice because I make mistakes; each mistake I recognize is an opportunity to learn, and thereby not make that mistake where it would be more expensive to do so.

Last night, I picked up the Fibonacci kata again.

I believe it was the Fibonacci kata where I finally got my first understanding of what Kent Beck meant by "duplication", many years ago.  So it is something of an old friend.  On the other hand, there's not a lot of meat to it - you go through the motions of TDD, but it is difficult to mine for insights.

Nonetheless, I made three significant errors, absolute howlers that have had me laughing at myself all day.

Error Messages

Ken Thompson has an automobile which he helped design. Unlike most vehicles, it has neither a speedometer, nor gas gauge, nor any of the other numerous idiot lights which plague the modern driver. Rather, if the driver makes a mistake, a giant "?" lights up in the center of the dashboard. "The experienced driver," says Thompson, "will usually know what's wrong."

In the early going of the exercise, I stopped to review my error messages, writing up notes about the motivations for doing them well.

Mid exercise, I had a long stare at the messages.  I was in the middle of a test calibration, and I happened to notice a happy accident in the way that I had made the test(s) fail.

But it wasn't until endgame that I finally discovered that I had transposed the expected and actual arguments in my calls to the Assertions library.

The contributing factors -- I've abandoned TestNG in favor of JUnit5, but my old JUnit habits haven't kicked back in yet.  While all the pieces are still fresh in my mind, I don't really see the data.  I just see pass and fail, and a surprise there means revert to the previous checkpoint.

Fence Posts

The one really interesting point in Fibonacci is the relationship between a recursive implementation and an iterative one.  A recursive implementation falls out pretty naturally when removing the duplication (as I learned from Beck long ago), but as Steve McConnell points out in Code Complete: recursion is a really powerful technique and Fibonacci is just an awful application of it.

What recursion does give you is a lovely oracle that you can use to check your production implementation.  It's an intermediate step along the way.

In "refactoring" away from the recursive implementation, I managed to introduce an off-by-one error in my implementation.  As a result, my implementation was returning a Fibonacci number, just not the one it was supposed to.

Well that happens, right? you run the tests, they go red, and you discover your mistake.

Not. So Much.

I managed to hit a perfect storm of errors.  At the time, I had failed to recall the idea of using an oracle, so I didn't have that security blanket.  I went after the recursion as soon as it appeared, so I was working in the neighborhood of fibonacci(2), which is of course equal to fibonacci(1).

My other tests didn't catch the problem, because I had tests that measured the internal consistency
of the implementation, rather than independently verifying the result.  One of the hazards that comes about from testing an implementation "in the calculus of itself."  Using property tests was "fine", but I needed one more unambiguous success before relying on them.

Independent Verification

One problem I don't think I've ever seen addressed in a Fibonacci kata is integer overflow.  The Fibonacci series grows roughly geometrically.  Java integers have a limited number of bits, so ending up with a number that is too big is inevitable.

But my tests kept passing - well past the point where my estimates told me I should be seeing symptoms.

The answer?  In Java, integer operators do not indicate overflow or underflow in any way. JLS 4.2.2

And that's just as true in the test code as it is in the production code.  In precisely the same way that I was only checking the internal consistency of my own algorithm, I also fell into the trap of checking the internal consistency of the plus operator.

There are countermeasures one can take -- asserting a property that the returned value must be non-negative, for instance.

Conclusions

One thing to keep in mind is that production code makes a really lousy test of the test code, especially in an environment where the tests are "driving" the implementation.  The standard for test code should be that it obviously has no deficiencies, which doesn't leave much room for creativity.


 

 

A study in ports and adapters

I've got some utilities that I use in the interactive shell which write representations of documents to standard out.  I've been trying to go fast to go fast with them, but that's been a bit rough, so I've been considering a switch to go slow to go smooth approach.

My preference is to work from the outside in; if the discipline of test driven development is going to be helpful as a design tool, then we should really be allowing it to guide where the module boundaries belong.

So let's consider an outside in approach to these shell applications.  My first goal is to establish how I can disconnect the logic that I want to test from the context that it runs in.  Having that separation will allow me to shift that logic into a test harness, where I can measure its behavior.

In the case of these little shell tools, there are three "values" I need to think about.  The command line arguments are one, the environment that the app is executing in is a second, and the output effect is the third.  So in Java, I can be thinking about a world that looks like this:


In the ports and adapters lingo, TheApp::main is taking on the role of an adapter, connecting the java run time to the port: TheApp::f.

For testing, I don't want effects, I want values.  More specifically, I want values that I can evaluate for correctness independently from the test subject.  And so I'm aiming for an adapter that looks like a function.

So I'll achieve that by providing a PrintStream that I can later query for information

In my tests, I can then verify, independently, that the array of bytes returned by the adapter has the correct properties.

Approaching the problem from the outside like this, I'm no longer required to guess about my modules -- they naturally arise as I identify interesting decisions in my implementation through the processes of removing duplication and introducing expressive names.

Instead, my guesses are about the boundary itself: what do I need from the outside world? How can I describe that element in such a way that it can be implemented completely in process?

Standard input, standard output, standard error are all pretty straight forward in Java, which somewhat obscures the general case.  Time requires a little bit of thinking about.  Connecting to remote processes may require more thought -- we probably don't want to be working down at the level of a database connection pool, for example.

We'll often need to invent an abstraction to express each of the roles that are being provided by the boundary.  A rough cut that allows the decoupling of the outer world gives us a starting point from which to discover these more refined boundaries.

Don't Skip Trivial Refactorings

Summary: if you are going to use test driven development as your process for implementing new features, and you don't want to just draw the owl, then you need to train yourself to refactor aggressively.

My example today is Tom Oram's 2018-11-11 essay on Uncle Bob's Transformation Priority Premise.  Oram uses four tests and seven distinct implementations of even?

If he were refactoring aggressively, I think we would see three implementations from two tests.

Kent Beck, in the face of objections to such a refactoring under the auspices of "removing duplication", wrote:

Sorry. I really really really am just locally removing duplication,
data duplication, between the tests and the code. The nicely general formula
that results is a happy accident, and doesn't always appear. More often than
not, though, it does.

No instant mental triangulation. No looking forward to future test cases.
Simple removal of duplication. I get it now that other people don't think
this way. Believe me, I get it. Now I have to decide whether to begin
doubting myself, or just patiently wait for most of the rest of the world to
catch up. No contest--I can be patient.

At that time, Beck was a bit undisciplined with his definition of "refactoring".  Faced with the same problem, my guess is that he would have performed the constant -> scalar transformation during his "refactoring" step.  If there isn't a test, then it's not observable behavior?

Personally, I find that the disciplined approach reduces error rates.