Premise
Consider the following method:
static String formatMyDate(Date date) {
return new SimpleDateFormat("yyyy-MM-dd").format(date);
}
It's often desirable to memoize DateFormat objects so they can be reused rather than repeatedly instantiating new ones. This frequently leads to the following naive refactor:
private static final DateFormat DATE_FORMAT = new SimpleDateFormat("yyyy-MM-dd");
static String formatMyDate(Date date) {
return DATE_FORMAT.format(date);
}
But this is wrong for multithreaded applications. From the DateFormat documentation:
Date formats are not synchronized. It is recommended to create separate format instances for each thread. If multiple threads access a format concurrently, it must be synchronized externally.
Assuming our application is using thread pooling, this leads us to memoize an object per thread, using ThreadLocal (see article):
private static final ThreadLocal<DateFormat> DATE_FORMAT_REF =
new ThreadLocal<DateFormat>() {
@Override
protected DateFormat initialValue() {
return new SimpleDateFormat("yyyy-MM-dd");
}
};
static String formatMyDate(Date date) {
return DATE_FORMAT_REF.get().format(date);
}
This works fine. But I've noticed it can still leave code duplication across a larger project. For example multiple developers could each write their own code using "yyyy-MM-dd" date formats in various classes. This led me to write the following helper class (using JSR-305 and Guava):
Solution 1
public class DateFormats {
private static final ThreadLocal<Map<String, DateFormat>> DATE_FORMAT_MAP_REF =
new ThreadLocal<Map<String, DateFormat>>() {
@Override
protected Map<String, DateFormat> initialValue() {
return Maps.newHashMap();
}
};
private DateFormats() { }
/**
* Retrieves, and if necessary creates and caches, a {@code DateFormat} instance
* corresponding to the specified format string.
*
* @param dateFormatString The date format string.
* @return The date format.
*/
public static DateFormat get(String dateFormatString) {
Preconditions.checkNotNull(dateFormatString, "dateFormatString");
final Map<String, DateFormat> dateFormatMap = DATE_FORMAT_MAP_REF.get();
@Nullable DateFormat dateFormat = dateFormatMap.get(dateFormatString);
if (dateFormat == null) {
dateFormat = new SimpleDateFormat(dateFormatString);
dateFormatMap.put(dateFormatString, dateFormat);
}
return dateFormat;
}
}
And the usage would look like:
static String formatMyDate(Date date) {
return DateFormats.get("yyyy-MM-dd").format(date);
}
One downside to this solution is that a new HashMap gets created for every calling thread. I'm considering using a Guava LoadingCache to implement the following alternative:
Solution 2
public class DateFormats {
private static final LoadingCache<String, ThreadLocal<DateFormat>> DATE_FORMAT_REF_CACHE =
CacheBuilder.newBuilder()
.concurrencyLevel(4) //how to compute this is up for debate
.build(new CacheLoader<String, ThreadLocal<DateFormat>>() {
@Override
public ThreadLocal<DateFormat> load(final String dateFormatString) {
return new ThreadLocal<DateFormat>() {
@Override
protected DateFormat initialValue() {
return new SimpleDateFormat(dateFormatString);
}
};
}
});
private DateFormats() { }
/**
* ...
*/
public static DateFormat get(String dateFormatString) {
Preconditions.checkNotNull(dateFormatString, "dateFormatString");
return DATE_FORMAT_REF_CACHE.getUnchecked(dateFormatString).get();
}
}
Questions
- Is the premise of this solution too pedantic, or is there value in implementing either solution?
- Which solution is preferable for a web-scale application?
- Is there anything about the solutions you would fix or otherwise change?
- For solution 2, what are ways to better calculate concurrency level?
I haven't had a chance to write unit tests, but will post them if requested when time allows.
