A couple of weeks back I worked on adding some new capabilities to a software system at work called the `listener'. Essentially, this software receives socket messages (events), assembles, parses (from various protocols), generates `model' events, and passes these off to a subsystem to let various business logic handlers deal with the events and do interesting things with them. The piping internal to the system to move events around includes (among other things) the Visitor Pattern, which is normally very explicit about not being able to handle a new type if a new type has been added.
The capabilities I needed to add depended on creating a new event type that the visitor pattern would pass to the appropriate handler on the back end (like visitor patterns do). I built the acceptance test, ran it, watched everything fail as I expected, then got into modifying the interfaces to force the creation of the new event type, and force the various implementations to implement a `visit' method for the new event. All of this went swimmingly, and after working for about three or four hours, I had the whole system back together with new UT's for the classes that needed changed, and with the original AT that started the whole thing working.
So, I had added about 50 UTs, and about 3 ATs and everything looked copacetic. I took the finished code off to run it on a test system with real data.
It didn't work.
After spending a couple of hours digging through the logs, tests, and finally the code, I found that when we built the system, we had added a base class beneath some of the implementations of the visitor for some reason. This base class provided default behavior for unknown types coming in. Since this base class was embedded in the plumbing, the default handler was taking the new event type, and wasn't letting it get to the endpoint implementation, so the new event wasn't being handled. Once I found that, I found several tests that I had missed on the first pass through the system that would have pointed out these problems, but I had glossed over them.
If I knew then what I know now, I'd:
- Make sure that if I'm using Visitor, I make the compiler warn me about problems. The fact that I'm using Visitor means that I'm SURE I'll be adding new types. Let the compiler do its job when the types aren't wired up correctly.
- If there is a compelling reason to stick a base class in there (i.e. a default visit handler), add a test that does reflection to count the number of visit types between the end point implementations and the interface. This might be a little hairy, but the 2-3 hours spent will save me several times in the future.
- If you're in a situation like this, and the base class is causing you pain, remove the base class.
It was a good learning experience. It just points out once again that hindsight is awesome.
No comments:
Post a Comment