How design rot

time to read 3 min | 535 words

image

I am currently doing some big modifications on the NH Prof code base, getting it ready for v1.0 status (and reserving some surprises as well).

Among other things, it means that I am trying to touch as much of the code base as possible.

It also means that I am finding stuff that I generally don’t see when using the application normally.

Here is a good example:

image

The internals of NH Prof are built on the notion of pub/sub, mostly because I wanted to be able to have good separation of concerns.

Take a good look at FormattedStatement. It is the reason for this post. Its design is about as rotten as you can get without having nasty liquids splashing off the keyboard every time it is used.

Again, this is an issue with historical changes. It used to be part of the following process:

image

There are other things going on there, like parameter extraction, alerts, etc.

Anyway, it started its life as a usual message, just a stupid data container, but it… evolved.

image

Obviously, this is no longer a message. But what to do about it? It is an important piece of the app, more so, with the changes that we are currently making here.

Well, for a start, it is no longer just a formatted statement, so I changed that to a SqlStatement. The second was to get that out of the Messages namespace and put it in the root backend model, where it actually belongs.

Now, it is being created as part of processing events from NHibernate, and I have some issue with that, since all the rest of the backend model is created in a later stage. But I think that this is acceptable. A statement is about the only part of the event stream that I don’t have to do correlations on, so it make sense not to create an additional causality layer on top of it, it is a single event, no need to correlate it.

Is this it? Didn’t I just said that it was a piece of rotten design? What did I do? Rename & change namespace! Is that enough to fix the design?

Well, yes. The reason that this was problematic is quite simple, it wasn’t in the right place, and it wasn’t doing what it stated it was doing. By moving it, we fixed those issues. To give a similar example, if you saw data access code in the UI, the action that you need to take is to move the data access code to the appropriate layer, that is all.