The question is wonderful (even if it does not correspond 100% to the site) , it is very nice to see that such efforts are made to have clean and maintained code in other places in the world too.
I would focus on the structures used, not the actual conversion, although I take into account thread safety - which is key when working with SimpleDateFormat . Therefore, to select the tool you need (for example, Joda-Time library, a mandatory recommendation on this topic)
Let's analyze the problem by taking a step back:
- you have fixed logic to apply to the same types of input and output, but the actual operation is performed in each case
- you want to apply a fixed set of operations
The first point invokes the Strategy pattern; it seems applicable here without any awkwardness or signs of patternitis.
public interface IDateFormatter { String format(Date d); Date parse(String s) throws ParseException; }
( ParseException fully applicable here semantically, and not just because this is what SimpleDateFormat throws ...)
The second point requires the use of an enumeration to store individual instances of the strategy. (I'm not sure, however, that limiting a set of elements can be called a strategy, because it is thus not extensible.)
Keep in mind that this exact code using instances of SimpleDateFormat is not thread safe! Instead of SimpleDateFormat this structure is sufficient to implement a stream safe conversion conversion.
public enum DateFormatter implements IDateFormatter { DATE_FORMAT(DATE_FORMAT), DATE_SHORT_FORMAT(DATE_SHORT_FORMAT), TIME_FORMAT(TIME_FORMAT), DATETIME_FORMAT(DATETIME_FORMAT), DATETIME_SHORT_FORMAT(DATETIME_SHORT_FORMAT); // for single threaded environments only! // (or substitute with thread safe implementation!) private final SimpleDateFormat format; private DateFormatter(String s) { format = new SimpleDateFormat(s); } public String format(Date d) { return format.format(d); } public Date parse(String s) throws ParseException { return format.parse(s); } }
However, to add another layer of abstraction, completely hiding the internal parts from the developers, you can wrap this using the API you provided (and, of course, in combination with the corresponding set of visibility modifiers and package settings):
public class MainDateFormatter<T extends Enum<T> & IDateFormatter> { //This stores the class for the enum, to private final Class<T> clazz; public MainDateFormatter(Class<T> clazz) { this.clazz=clazz; } // Format using dd-MM-yy public String asShortDate(@Nullable Date date){ return DateFormatter.DATE_SHORT_FORMAT.format(date); } // Parse using dd-MM-yyy public Date parseShortDate(@Nullable String string) throws ParseException; return DateFormatter.DATE_SHORT_FORMAT.parse(string); } //... and the rest similarly }
You also mentioned Spring: you can put this object in an application container with a Singleton scope and be sure that no one can use anything else - as this will facilitate access.
Scary multithreaded
SimpleDateFormat (and DecimalFormat , for that matter) are known to be non-streaming. In addition, ThreadLocal associated with some concerns, but in this case I would not go against them, since currently there is no "magic" here, and classes are loaded using the same instance of ClassLoader.
public enum DateFormatter implements IDateFormatter { //... instances... // for multithreaded environments // BEWARE ClassLoader magic can break this leading to memory leak! final ThreadLocal<SimpleDateFormat> formats; //note the final! private DateFormatter(final String s) { formats = new ThreadLocal<SimpleDateFormat>() { @Override protected SimpleDateFormat initialValue() { return new SimpleDateFormat(s); } }; } public String format(Date d) { return formats.get().format(d); } public Date parse(String s) throws ParseException { return formats.get().parse(s); } }
(bits and pieces taken from this answer )
How safe are the values of SimpleDateFormat and DecimalFormat without a ceiling?
When I run this in parallel (100,000 threads), I see a 3.5% failure, with unusual error messages, I think this is because it is not thread safe.
One of my former colleagues did an experiment: 2 threads were enough to spoil the conversion, and get output, for example , February 31st SimpleDateFormat , and a strange output not even related to input numbers with DecimalFormat . Not surprisingly, Sonar warns of their entry into static final variables ...
The bad news is that most errors will go unnoticed until someone complains ... If strange error messages appear on the surface, this is a completely different level of the problem.
Dealing with this may include the use of ThreadLocal s (or pools of objects, if not too many service ones) if you see that creating format objects is a bottleneck, or if it is not, simply by creating "short live", strictly local instances. when the conversion will happen.
Clean API
At first, I had conflicting feelings, since your proposed API has many methods with the same signature; all use the same input to provide the same output, they share the logic. I really wrote a small class to “fix” this:
public class DateFormatterHider<T extends Enum<T> & IDateFormatter> { private final Class<T> clazz; public DateFormatterHider(Class<T> clazz) { this.clazz=clazz; } public String format(Date d, String formatCode) { return getEnumInstance(formatCode).format(d); } public Date parse(String s, String formatCode) throws ParseException { return getEnumInstance(formatCode).parse(s); } private T getEnumInstance(String formatCode) { return Enum.valueOf(clazz,formatCode); } }
But after I wrote this, now it was not so nice:
- this requires string constants (I don't like them.) that need to be placed somewhere
- The parse method has the signature
parse(String,String) - it can irritate the general smell
It seems like the advantage is that it allows you to replace the implementation by providing a different enum class (for example, only in Spring XML), but this quickly turns into a nightmare: since the format code is String , it does not provide any way to track dependencies. If someone changes the enumeration and falsely names the renaming name, the code can only ever fail on Christmas night, while singing O Tannenbaum (*) with children who can quickly get two endings: getting fired, or a painful and long divorce. The code is dangerous!
So, this is - while the impressively confusing "geekish" solution, including tricky generics - is definitely no-no.
A readable, intuitive interface with the right dependencies is critical to providing a supported application.
Perform date type conversions
Developers are inventive. Another word for this is lazy - this is an undesirable view of the same. Having rules to follow often, as a rule, leads to a kind of ingenuity, providing real ways to circumvent the rules, so I would create rules in your continuous integration system to check that, for example, SimpleDateFormat not used anywhere in the code. If you need clean code over time (i.e. goooood), it should be at least a critical error. I would say that it won’t even hurt, so that he couldn’t build ...
Format guessing option
All your formats are unique in length. This makes it technically possible for this exact situation to create a procedure for guessing the format (BTW for those who read this far, would like to get b33r, I think ...), which tries to choose the correct format depending on the length of the input string and provide a method. taking one line, and still working correctly.
However , for me it belongs to the category of hidden magic . Perhaps it will be much more readable and supported so as not to have this function, since this will lead to errors that go unnoticed (for example, the data comes in the wrong format, etc.), which no one wants! (Christmas night, O Tannenbaum, children, you know the rest ...)
*: not Oh Tanenbaum , this is another story