Paul Hammant's Blog: Bad Java servlet apps
Here’s a recipe for making really bad Java server apps. Perhaps ones that verge on unmaintainable as the years pass. Patterns and practices have evolved over 18 years of enterprise Java. Every now and then, there is something that developers declare is the promised land, and a lot of intellectual effort is spent on it. Sometimes frameworks emerge. Maybe just “best practices”. When we look back years later, we cringe. I’m going to enumerate bad decomposition, dependency management, and state tracking techniques (and technologies).
These are wrong because they:
- are inordinately hard to understand if you did not write it yourself
- hard to debug
- hard to unit test
- increase entanglement
Team agreement that these are wrong means:
- 2x, 4x or 8x more expensive functional changes.
- progressive team de-skilling as the talented vote with their feet
- mounting executive dissatisfaction
Choose more than one for really really bad applications:
Looking back ten or so years, Struts is terrible. No wonder Ruby on Rails was able to claim “three to five times faster” in respect of developer throughput. There’s so much indirection, verbosity and needless configuration in it, that it’s a wonder that anyone mastered it. Hopefully it is fully gone in this age. If not, you should really question why not. If there are applications that still rely on it, that still have development teams I find myself shocked.
Apart from that indirection, verbosity, and boiler-plate, the XML configuration is massively over engineered. If you’re designing something that has externalized configuration, you should ask yourself “is someone going to edit this on the production machine directly” ? If the answer is no, or pretty close to never, whatever was encoded in that externalized configuration should be in Java (or Ruby, C# etc) and you should gain experience with re-deploying the application for such things.
The classic design pattern I mean -
ZipCodeService.getInstance().isAZipCode(zipCode), not the Spring or Guice idiom.
This is what Inversion of Control (1998 onwards) Dependency Injection (2003/4 onwards) was railing against, and nightmare of entanglement that large applications, on their own, could foster.
ZipCodeService.INSTANCE.IsAZipCode(zipCode) // DotNet
The pattern like so:
((ZipCodeService) OurServiceLocator.getInstance().get("ZipCodeService")).isAZipCode(zipCode). or since Java5
Subverting the Dependency Injection container
This is either deliberately storing the dependency injection container somewhere globally, so that other objects can use it, or taking advantage of that technologies own mechanism to invert the inversion of control.
An example of the latter is Spring’s
WebApplicationContextUtils.getRequiredWebApplicationContext(servletContext) which should never be used IMO (ignoring perceived problems inherent in the Java Taglib technology, for lovers of Dependency Injection).
Dependency Injection containers use ThreadLocal. They kinda have to because of the separation of filters, servlets and context-listeners.
Application developers can also decide to use them to pass state or functionality around the application without having to declare constructor/method parameters. Perhaps they should not.
Too much reliance on Servlet filters chaining.
The servlet spec itself allows filters to choose to delegate to successive ones in a chain, or ‘complete’ without that delegation. For a multi-year project each new team, that is asked to add functionality, might think they can add a new filter in front of all those previously declared. As with other techniques, its a way of getting the job done, but it’s hardly adding clarity to the code long term.
Visiting set/getAttribute too many times for one request
This could be an attempt at caching via
servletContext.setAttribute(key, value) or it could be a convenient place to store per-request state in one request (via
servletRequest.setAttribute(key, value)). Either way, multiple ‘action’ method invocations can read/write to there, as part of a larger orchestration. It’s too convenient, especially when it’s the same items that get/set multiple times in one request. Lifecycle starts to get confusing aside from anything else.