Cron expression validator for Apache Quartz

Question

Not too long ago, I had to create a cron expression for a route in Apache Camel. I had a bit of a struggle to find the right expression. So I made a small program to output the n next valid dates returned by the expression. I planned to add more feature like instead of making a blind Sysout, you could create a file with your dates.

In my application, a cron expression correspond to the definition of this class. It's use to fire an event for a Camel route. You will set a Quartz endpoint to your route and use a cron expression to express at which interval it should start. Some example :

0 0/5 12-18 ? * MON-FRI

This cron expression translated to a more human format would be : every five minutes starting at 12pm (noon) to 6pm on weekdays. (this cron is taken from the quartz documentation).

For this particular cron, my program (with an numberOfdates set to 5) would output :

Fri Sep 19 12:05:00 EDT 2014
Fri Sep 19 12:10:00 EDT 2014
Fri Sep 19 12:15:00 EDT 2014
Fri Sep 19 12:20:00 EDT 2014
Fri Sep 19 12:25:00 EDT 2014

The application is really basic. I take two arguments, the cron expression and the numbers of date you want outputted.

Feel free to cover any aspect. I had particular problem with the naming of my classes and variables. There isn't much code at the moment, but my naming and the way I verify arguments worry me a bit.

It's a public repo on GitHub

CronTester

import java.util.Date;
import java.util.List;
public class CronTester {
 /**
 * The application take two arguments : 
 * The first argument is the cron expression surrounded by " : "* * * * * *"
 * The second argument is the number of dates you want to verify. It's a simple Integer.
 * @param args
 */
 public static void main(String[] args) {
 if(args.length > 2) {
 throw new IllegalArgumentException("You should only provide 2 arguments. \"[cronExpression]\" [numberOfDates]");
 } else if (args.length != 2) {
 throw new IllegalArgumentException("You need to provide 2 arguments in the following form : \"[cronExpression]\" [numberOfDates]");
 }
 final String expression = args[0];
 final int numberOfDates;
 try {
 numberOfDates = Integer.parseInt(args[1]);
 } catch (NumberFormatException e) {
 throw new IllegalArgumentException("The second argument need to be a valid integer.");
 }
 final CronDateCreator dateCreator = new CronDateCreator(expression);
 List<Date> result = dateCreator.createValidTimeDatesFromNow(numberOfDates);
 for(Date expected : result) {
 System.out.println(expected);
 }
 }
}

CronDateCreator

import java.text.ParseException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import org.quartz.CronExpression;
public class CronDateCreator {
 private CronExpression cron;
 public CronDateCreator(String cronExpressionRaw) {
 try {
 this.cron = new CronExpression(cronExpressionRaw);
 } catch (ParseException e) {
 throw new IllegalArgumentException("The cron expression supplied is not a valid cron expression.",e);
 }
 }
 public List<Date> createValidTimeDates(Date startingDate, int numberOfDates) { 
 Date nextValidDate = cron.getNextValidTimeAfter(startingDate);
 List<Date> results = new ArrayList<>();
 for (int i = 0; i < numberOfDates; i++) {
 results.add(nextValidDate);
 nextValidDate = cron.getNextValidTimeAfter(nextValidDate);
 }
 return results;
 }
 public List<Date> createValidTimeDatesFromNow(int numberOfDates) {
 return createValidTimeDates(new Date(), numberOfDates);
 }
}

• rolfl: Are you running on Java 8, or are you restricted to <= Java7?
• Marc-Andre: No restriction!

Answer 1

In terms of overall hierarchy here, the CronTester class is just 'fluff' and does not do anything except provide an main method. The interesting class is: CronDateCreator.

CronDateCreator is a very lightweight class. It essentially does nothing that requires it to be a class... it wraps a String value in a CronExpression, is that CronExpression reused for anything?

It strikes me that the standard use-case for this class would be:

CronDateCreator cdc = new CronDateCreator(input);
List<Date> dates = cdc.createValidTimeDatesFromNow(numberOfDates);

and then the cdc will be forgotten.

Is this a case of where a static method is maybe all you need?

Something like:

public static List<Date> createValidTimeDatesFromNow(String expression, int numberOfDates) {
 ....
}

Just a thought...

Date

Now, about the Date return values.

Date is a much maligned class in Java. It has so many nuances, issues, and tweaks that it was initially 'forked' and rewritten as JodaTime in an open source library, then, in Java8, they have pulled the best from JodaTime, and incorporated some other compatibility and functionality changes, and created the new java.time.* package.

Any code written to support Java8 or newer should essentially abandon Date, and work with Instant instead.

You should use your class/static methods to translate the Dates back to Instants, and go from there.

Actual code:

Your method here could be neatened up a lot:

public List<Date> createValidTimeDates(Date startingDate, int numberOfDates) { 
 Date nextValidDate = cron.getNextValidTimeAfter(startingDate);
 List<Date> results = new ArrayList<>();
 for (int i = 0; i < numberOfDates; i++) {
 results.add(nextValidDate);
 nextValidDate = cron.getNextValidTimeAfter(nextValidDate);
 }
 return results;
}

For a start, you know how large the ArrayList will be, so set the size on the constructor:

List<Date> results = new ArrayList<>(numberOfDates);

Then, your loop logic is off... how about the following?:

public List<Date> createValidTimeDates(Date startingDate, final int numberOfDates) { 
 List<Date> results = new ArrayList<>(numberOfDates);
 
 for (int i = 0; i < numberOfDates; i++) {
 startingDate = cron.getNextValidTimeAfter(startingDate);
 results.add(startingDate);
 } 
 return results;
}

• Marc-Andre: At first everything was in the same class, so I moved the cron method into his own class. I guess it's because at that time I was thinking in adding more functionality. I'll move from Date, Java 8 is not something we're working with at work, so I'm a bit "late" with all the new stuff.
• maaartinus: @Marc-Andre And so am I, but I can repeat the recommendation against Date. It's maybe the worst class ever. There's an official backport of JDK 8 time library.