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! :)
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! :)
Labels: dev
4 Comments:
At June 25, 2008 12:02 AM ,
raveman said...
i agree that "Lots of 'Helper' or 'Util' classes indicates a poor design", but I disagree with your idea. Java has some poor designs, but i think you should not hack it. When you add another framework you will have to hack it and maybe you wont be able (if it checks class type and not instance of). Plus if I see MyDate in code i know i cant fully trust it and when errors come it might be a problem with MyDate implementation. I think the real problem is when people dont use util classes, but use ugly api. for example String format = new SimpleDateFormat("yyyy-MM-dd").format(new Date()); Its easy to misspell "yyyy-MM-dd" and even why remember that MM is with big letters? it might be usefull for job interview(many people are pround that they remember that), but its code duplication. I use only 3 types of date formats, so with util class i have const or 3 methods. I think String DateUtil.formatyyyyMMdd(Date) is much better then orginal. All util classes could get deprecated if only java would get better design.
At June 25, 2008 7:53 PM ,
Mwanji Ezana said...
Thanks for your comments.
"Java has some poor designs, but i think you should not hack it"
Is extending a non-final class "hacking" it? Also, note that I'm strictly adding application-specific behaviour, and no state, as the Date class has fairly complex state management already, that could be easy to mess up.
"When you add another framework you will have to hack it and maybe you wont be able (if it checks class type and not instance of)."
Well, that's the framework's fault, not mine. Frameworks used to force getters and setters on everyone, and now they don't anymore. I try to avoid Framework-Dictated Development as much as possible (hence the use of Hibernate type converter). If you're somehow forced to use a framework that doesn't agree with your design style, then you'll have to make sacrifices.
"Plus if I see MyDate in code i know i cant fully trust it and when errors come it might be a problem with MyDate implementation."
Obviously, when you extend core classes, you have to tread carefully, and I definitely don't recommend using it all the time. For more complex class (a Map, for example), composition is probably a better idea.
"I think String DateUtil.formatyyyyMMdd(Date) is much better then orginal."
So do I, I just think we can go one step further.
At June 26, 2008 3:33 PM ,
Hamlet D'Arcy said...
In practice, providing extra functionality using inheritance in Java (as you suggest) has not worked out well for me at all. Have you looked at the PHP String functions documentation? (Search Google). The API is hideously cluttered. Do you really need a levenstein and soundex method on every string in your system? There is no separation of concerns here or cohesion. This is just a bunch of unrelated functions that work on strings... not a good decomposition of types.
Well, extending Date (or adding DateUtils) is making the same design choice. You are not decomposing your design into coherent types. Instead, you're decomposing them into tightly coupled objects. Sure, your approach has the advantage of not being a compile time coupling. But it is all still there.
I highly recommend the Adapter Pattern for Java. Define small, coherent interfaces, and then delegate to string instances under the covers.
At June 29, 2008 11:22 AM ,
Mwanji Ezana said...
Hi Hamlet,
Thanks for the comment, you make a lot of good points. I was actually planning a follow-up using composition rather than extension, I'll respond to your arguments there.
Post a Comment
Links to this post:
Create a Link
<< Home