3

I am working on this project. There is a class DefaultsHelper which has : 

    public  class DefaultsHelper {

    static SimpleDateFormat df = new SimpleDateFormat("yyyy-MM-dd") ;

    public static String getDate(int days)
    {
        GregorianCalendar c = new GregorianCalendar() ;
        c.setTime(new Date()) ;
        c.add(Calendar.DAY_OF_MONTH, days);
        return df.format(c.getTime()) ;

    }
}

In a web app - if two users where viewing a jsp whereby the getDate function was called at the exact same time - is it possible that the df object could get inconsistent and thereby not return expected values?

I think the DefaultsHelper was meant to be a utility class to stop having to instantiate new df objects

user1882639
  • 45
  • 4

1 Answers1

5

You create a new Calendar per thread, which is good, but you're using the SimpleDateFormat across multiple threads. This would be ok if that class were thread-safe, but it's not.

SimpleDateFormat is notorious for being thread-unsafe. Simply create a new one for each invocation (i.e. in the method), or, better still, use Joda-Time to avoid thread issues completely.

On the subject of creating one formatter class as an optimisation, this is a classic example of premature optimisation and inadvertent results. Given that this is within a JSP, the HTTP request/response time will dwarf the instatiation time (and resource requirements) of a new formatter. Make your classes thread-safe and immutable and worry about optimisation when it becomes an issue.

Community
  • 1
  • 1
Brian Agnew
  • 268,207
  • 37
  • 334
  • 440
  • thanks i thought that might be the case - i would be better off instantiating a new SimpleDateFormat inside this method then or change how the whole thing works ! Saved me a bad one as times in dbase were written this way – user1882639 Dec 06 '12 at 14:32
  • You will enhance your thread safety massively by searching for static instances of SimpleDateFormat within your applications! – Brian Agnew Dec 06 '12 at 14:40
  • thanks thats great i changed static like so - look ok ? public static String getDate(int days) { GregorianCalendar c = new GregorianCalendar() ; c.setTime(new Date()) ; SimpleDateFormat dft = new SimpleDateFormat("yyyy-MM-dd"); c.add(Calendar.DAY_OF_MONTH, days); return dft.format(c.getTime()) ; } – user1882639 Dec 06 '12 at 15:40