Use extension to avoid utility class proliferation

posted by Moandji Ezana

Neal Ford claims that "Lots of 'Helper' or 'Util' classes indicates a poor design" and suggests trying to "incorporate your helpers into an intelligent domain object." If the projects I've been on are any indication, his advice hasn't been heard, and people end up with, for example, a DateUtil class that provides calculations, formatting and all that other stuff that the JSE libraries are so terribly unwieldy for. I'm sure you've come across code like:

long DateUtil.calculateTimeBetweenDatesInSeconds(Date, Date)

and

String DateUtil.formatyyyyMMdd(Date)

or even (gulp):

date1.getTime() >= date2.getTime()

This kind of code is extremely common, but, let's face it, pretty ugly, not really object-oriented and, most importantly, difficult to read and error-prone. I'd be much happier writing:

boolean Date.isBefore(Date) [No more "Which date goes first, again?"]

String Date.formatyyyyMMdd()

long getIntervalInSeconds(Date)

So, here's an attempt to apply Ford's advice.

Begin by extending java.util.Date and copy the truly necessary DateUtil methods (I've found that utility classes have a tendency towards bloat) to it. That's pretty easy, but it's when you try to integrate your fancy new MyExtendedDate with clients and frameworks that things get a little trickier.

You could simply change the type of the domain object's field and modify the getters and setters [1]. However, if you're using an ORM framework, it might not be too happy about that, as it doesn't know MyExtendedDate. You'd have to teach it about your class. For example, with Hibernate you'd use a custom type converter. If you're using Struts2 and navigating your object graph with OGNL [2], you may have a similar problem, which you'd resolve in a similar way.

If you're mapping getters and setters directly onto your fields [3], your new setter will break clients. You could simply change setDate(MyExtendedDate) back to setDate(Date), because the subclass doesn't add any state, only behaviour, then create a new MyExtendedDate instance based on the date passed in. Or, if possible, keep setDate(MyExtendedDate) and change all its clients. Or have both and mark the former deprecated. The ideal solution, of course, would be getting rid of the setter altogether [3].

If you can't change the type of the field, you could change getDate() to return MyExtendedDate, which has the advantage of not breaking clients. If you can't change the field's type, you probably can't get rid of setDate(Date), either, but clients can give it a MyExtendedDate anyway, so it's not too much of an issue ([3] notwithstanding).

All that now remains is to find all the calls to DateUtils, replace them with MyExtendedDate's methods and eventually get rid of DateUtils. If you can't remove DateUtils right away, you could at least deprecate it and reimplement its methods to call the corresponding ones on MyExtendedDate.

Voilà: nice, smart and object-oriented Dates, at your service! If you're interested in more ways to improve your code without breaking everything, I highly recommend Michael Feathers' book Working Effectively With Legacy Code. It's like an extension to Martin Fowler's Refactoring that deals with all those real-world situations that make your eyes bleed.

[1] Getters and setters are evil, of course, but that's another topic, see Peter Gillard-Moss for a possible alternative. The last line, paraphrased, sums it all up nicely: "The trick with maintaining your encapsulation as neatly as possible is to try and ensure that [you] deal with the concepts of [the] domain (for example IStatistic) and not the structure of the data (e.g. int speed)."

[2] If you have a big domain model, I highly discourage this, as it leads to unrefactorable and tightly-coupled code. Another topic for another time.

[3] Remember: getters and setters are evil! :)

JUnit and auto-boxing

posted by Moandji Ezana

After that gigantic previous post, I thought it'd be good to post something simpler and more widely applicable.

I came across a little JUnit assertion gotcha that had me puzzled for a second. I was testing this (note, calculateDurationInSeconds() returns a long)

assertEquals(120, subjectUnderTest.calculateDurationInSeconds());

and getting the following, less-than-helpful test failure message

expected: <120> but was: <120>

I figured this one out pretty quickly because my Eclipse underlines cases of auto-boxing/unboxing. Since the assertEquals(Object, Object) method is being used, my int is becoming an Integer and the long returned by the method is becoming a Long. Obviously, any equals() comparaison is going to fail. I corrected this by using the long literal notation:

assertEquals(120L, subjectUnderTest.calculateDurationInSeconds());

What with all the conversion already going on between the presentation and data layers, I'm trying to generalise the practice of making primitive/wrapper conversions explicit, just in case.

How Tomcat ruined my night

posted by Aymeric Levaux

Tomcat is probably one of the more widely used pieces of open source infrastructure and it's a great boon to our productivity. Sometimes, though, a deep-rooted problem can sneak up on you and drag you into a sleep-deprived night of frantic web searching.

One day, I happened to see our production JVM restart, for no apparent reason. Intrigued, I looked into it a little bit more and realised that since the release of JBB v3 last December, it had been restarting roughly 6 times per month. After an OutOfMemory, the JVM dumps its heap into an hprof file, so there were a number of these files on the server, often over 1GB each!

I downloaded an hprof file and tried to open it with jhat, provided with the default Java 6 distro. jhat creates a website for you to visualise the heap [1]. Unfortunately, I couldn't get jhat to work, probably because JavaScript support is not yet implemented in MacOSX's JDK 1.6. I got around this problem by reading the hprof with YourKit Java Profiler 7.0 and quickly saw that vast amounts of memory were being taken up by several huge char arrays (click on the image for a larger version).



As you can see from the screenshot, 115 instances of org.apache.jasper.runtime.BodyContentImpl were holding on to over 560 million bytes worth of objects, keeping them from being garbage-collected and eating up inordinate amounts of RAM. So, what's BodyContentImpl actually doing?

Tomcat maintains a pool of PageContext instances, which in turn have an array of BodyContentImpls. Each BodyContentImpl has an array with the text from the tag's body. This array has a default size of 512 that is hard-coded as org.apache.jasper.Constants.DEFAULT_TAG_BUFFER_SIZE. Once the body of a custom tag gets bigger than that, the array grows and is never reset to its original size. With concurrent users, several such arrays might be created, so if a few popular pages use big custom tags, you can end up with a number of gigantic, un-garbage-collectable char arrays. In our case, this happened because in v3 we started decorating large blocks of JSP code with SiteMesh custom tags.

One solution would be to set the environment variable org.apache.jasper.runtime.BodyContentImpl.LIMIT_BUFFER to true, so that arrays over 512 bytes are released. Another would be to recompile Tomcat with a tag buffer size more suited to our needs. So both choices are extreme: either you maintain huge arrays for ever, or you're constantly creating and garbage collecting arrays, as 512 bytes is a fairly low limit for custom tags. We settled on giving the JVM more RAM to handle the extra garbage collection.

Of course, others have experienced the same problem. What's the word from the Tomcat camp? Well, Remy Maucherat, Tomcat committer, considers that:

Using sensibly written software helps. It should be obvious reading the API that using large body tags is going to be a huge problem.

Please draw your own conclusions.

1. By the way, jhat loads the whole hprof file into memory, so you may need to use the -J-mx512m flag if you're having OutOfMemory thrown.