0

I'm supporting Java 1.6 code where a date comparison is done.

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

static void process(Message message) {
 String now =formatter.format(new Date());
 String end = formatter.format("01-12-2017");

 Date endDay = formatter.parse(end);
 Date currentDay = formatter.parse(now);

 if(endDay.before(currentDay)){
    System.out.println("Send a notification indicating the end day has been reached, so the message is not processed.");
 }else {
    System.out.println("Process the message and applies some business rules");
 }
}

I know the code is not the best, but of 160,000 transactions 3 have failed and the code in the if block is executed. I'm planning use Calendar, but What could have happened here?

Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
mlavarreda
  • 30
  • 5
  • If you can use Java 8 `java.time.*` I strongly recommend that. – Simon Feb 13 '17 at 15:17
  • Thank you Simon, I'm supporting a Java 1.6 platform. – mlavarreda Feb 13 '17 at 15:18
  • Then maybe Joda time is an option – Simon Feb 13 '17 at 15:19
  • 7
    While changing API would definitely be a good thing in general, there's no reason why `Date.before` should fail. It's not clear what your expected behaviour is compared with actual behaviour. Please provide a [mcve]. Note that you're currently relying on the system default time zone, which isn't a great plan... – Jon Skeet Feb 13 '17 at 15:22
  • 2
    Is this the actual code? Is the end day really hardcoded? do you ever run this close to midnight? Are all the machines where this code runs in the same timezone? – Simon Feb 13 '17 at 15:26
  • @JonSkeet I also suggest a Timezone issue. The now is switched to *30-11-2017* because of the timezone offset and then`endDay.before(currentDay)` is true. – Alexander Feb 13 '17 at 15:37
  • Thank you @JonSkeet I'll try to run a simple test on the server within a main method running this example a couple millions times and see what is the result. – mlavarreda Feb 13 '17 at 18:10
  • @Simon the code is executed every time a Message is received. So it can happens at anytime during the day. The 3 failed transactions have happened at different days at different times. Operations received seconds before and after those failed operations were well executed. – mlavarreda Feb 13 '17 at 18:15

2 Answers2

4

tl;dr

Use the thread-safe java.time classes instead of the thread-unsafe legacy SimpleDateFormat class.

DateTimeFormatter f = DateTimeFormatter.ofPattern( "MM-dd-uuuu" ) ;
ZoneId z = ZoneId.of( "America/Montreal" ) ;
…
LocalDate.parse( "01-12-2017" , f )
         .isBefore( LocalDate.now( z ) )

Legacy date-time classes not thread-safe

I noticed you are using a singleton for the formatter, a single static instance of SimpleDateFormat. That legacy class is not thread-safe, as documented on the class JavaDoc:

Synchronization

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.

Your report of occasional error for seemingly no good reason suggests to me that you are using that object from more that one thread at a time. The static method named process, and your mention in comments of a server, makes me even more suspicious. Once in a while during runtime you experience a collision between threads.

Avoid legacy date-time classes.

You are using troublesome old date-time classes that are now legacy, supplanted by the java.time classes. Much of the java.time functionality is back-ported to Java 6 & 7 in ThreeTen-Backport.

The java.time classes use immutable objects, and are designed to be thread-safe. So says the package documentation.

LocalDate

You are using date-time objects for a date-only value. The java.time classes include a way to represent a date-only. The LocalDate class represents a date-only value without time-of-day and without time zone.

A time zone is crucial in determining a date. For any given moment, the date varies around the globe by zone. For example, a few minutes after midnight in Paris France is a new day while still “yesterday” in Montréal Québec.

Specify a proper time zone name in the format of continent/region, such as America/Montreal, Africa/Casablanca, or Pacific/Auckland. Never use the 3-4 letter abbreviation such as EST or IST as they are not true time zones, not standardized, and not even unique(!).

The ZoneId class is thread-safe. So you may retain an instance for repeated use across threads.

ZoneId z = ZoneId.of( "America/Montreal" );
LocalDate today = LocalDate.now( z );

When representing dates as strings, I suggest using standard ISO 8601 formats. For date-only that would be YYYY-MM-DD such as 2017-01-23. The java.time classes use these standard formats by default when parsing/generating strings.

LocalDate target = LocalDate.parse( "2017-01-23" );

If the format is out of your control, use DateTimeFormatter as shown in many other Questions & Answers on Stack Overflow. This class is thread-safe, so you can retain an instance for repeated use across threads.

DateTimeFormatter f = DateTimeFormatter.ofPattern( "MM-dd-uuuu" );
LocalDate target = LocalDate.parse( "01-12-2017" , f );

Compare with methods such as isEqual, isBefore, and isAfter.

if( target.isBefore( today ) ){
    System.out.println( "Send a notification indicating the end day has been reached, so the message is not processed." );
} else {
    System.out.println( "Process the message and applies some business rules" );
}

About java.time

The java.time framework is built into Java 8 and later. These classes supplant the troublesome old legacy date-time classes such as java.util.Date, Calendar, & SimpleDateFormat.

The Joda-Time project, now in maintenance mode, advises migration to the java.time classes.

To learn more, see the Oracle Tutorial. And search Stack Overflow for many examples and explanations. Specification is JSR 310.

Where to obtain the java.time classes?

Community
  • 1
  • 1
Basil Bourque
  • 303,325
  • 100
  • 852
  • 1,154
  • Thank you very much for your complete and detailed answer!!. You were absolutely right, the problem was the use of thread-unsafe SimpleDateFormat. I perform a test creating a Executor thread pool of 5 threads and I was able to reproduce the error. – mlavarreda Feb 23 '17 at 10:45
0

You see Javas Date library is not known to be the best there is for working with dates. Especially when we are talking about java-1.6.

My suggestion to you is to get in touch with this Joda Time. Here you can find all you need for the library. In my personal experience it works way more stable and it is easy to understand.

Leonardo
  • 34
  • 2
  • 3
    This doesn't actually address the question being asked. Sure, Joda Time is a better API - but `Date.before()` works, in itself. This would be appropriate as a comment, which I know you can't add yourself - but there are already comments suggesting using Joda Time. – Jon Skeet Feb 13 '17 at 18:12
  • Yep, Date.before() works. The problem was the SimpleDateFormat as singleton. – mlavarreda Feb 23 '17 at 10:53