Refactoring - How did we end up in this mess?

Peter Harrison - Jan 24 '20 - - Dev Community

Skink

Over the last few weeks I have been doing a bit of refactoring of a Java webapp. It isn't like I don't know how things should be done, but somehow expediency creeps in and we end up doing what gets the job done quickly rather than making the right decisions.

Early architectural decisions are difficult to change.

Some really nasty code comes from deliberate decisions I made years ago to couple the core domain classes to special cases rather than a more general and thus simple object. The logic at the time was that I didn't want it to be more general than necessary. This leads to special handling for these special cases almost everywhere. When the system was originally developed there was actually a technical basis for this decision, but since then we have rewritten the core storage and those technical reasons no longer apply. But to maintain API compatibility these special cases persisted.

The tar pit of class dependencies.

When I first was told about Spring I didn't understand what it was trying to be. But once you get the religion it is a powerful tool. I wasn't convinced that the REST controllers need to be essentially veneers over the real services, so we have controllers implementing direct calls to the repository interfaces using Mongo Spring. But sometimes a controller will need to access the functionality in another controller. As a result we got into the habit of creating dependencies between controllers.

This is a really bad idea. It creates a knot of dependencies, or a dependency cycle which makes it very difficult to test and susceptible to Spring dependency cycles which are difficult to resolve.

We are now refactoring this by pulling down methods which need to be accessed from multiple controllers into service classes. These service classes give us internal access to the methods without needing to depend on controllers. Do I hear a 'told you so'?

Static or not to Static?

The orthodoxy of Spring as far as I understand it is that you should not use static classes or methods. If you do it makes the inversion of control harder, and you can't mock anything because of the bindings. As a result I pretty much never used static methods or classes.

With the rise of other programming concepts I'm now moving back to using static classes and methods where it makes sense. This is driving back toward more of a functional programming approach, although the size of the code base means we are not about to do wholesale rework.

Easy in hindsight.

There have been several cases where the elegant solution to a problem was not evident and we ended up writing something which worked for a very narrow case, only to have the elegant solution become evident and easy later. However, because the existing code is in production and working it is difficult to justify the time and effort of reworking it. Very annoying. Luckily the most annoying case might go away soon.

Everyone sucks.

Even when you really try to hold the line against the raging storms of competing requirements and quality we can't always steer the straight and narrow course. It is more important to admit that we are not perfect, but to evaluate our work and do what we can to both refactor existing projects as we work and to learn from the experience. So what anti-patterns have you found creeping into your code? How did you deal with it?

. . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . . .