Cfrtjryk

I'm looking to increase the conciseness of this code. I realize that I can use Joda (or Java 8's new date API), but if I were to keep this to just Java 7, any suggestions? (I care less about whitespace and formatting.)

/**

 * Tests whether the date input represents

 * a real date in mm/dd/YYYY format that is after the current date. 

 * Useful for testing send dates and expiration dates.

 *

 * Tests against current date minus 24 hours

 * so users in different time zones from the server can

 * be accommodated.

 *

 * @param pDateString  date to be tested

 * @return true if date (@12:01 am) >= yesterday (@12:02am)

 */

public static boolean isValidFutureDate(String pDateString) {

    if (!isValidDate(pDateString)) {

       return false;

    }

    StringTokenizer st =  new StringTokenizer(pDateString.trim(), "/");

    if (st.countTokens() != 3) {

       throw new NumberFormatException("Date format should be MM/DD/YYYY.");

    }

    String month = st.nextToken();

    String day = st.nextToken();

    String year = st.nextToken();

    long oneDayInMillis = 86400000;

    // set reference point to yesterday's date, 12:01am

    GregorianCalendar ref = new GregorianCalendar();

    ref.setTime(new Date(System.currentTimeMillis() - oneDayInMillis));

    ref.set(Calendar.HOUR_OF_DAY, 0);

    ref.set(Calendar.MINUTE, 1);

    ref.set(Calendar.AM_PM, Calendar.AM);

    // set comparison time to entered day, 12:02am

    GregorianCalendar now = toDate(year, month, day);

    now.set(Calendar.HOUR_OF_DAY, 0);

    now.set(Calendar.MINUTE, 2);

    ref.set(Calendar.AM_PM, Calendar.AM);



    return now.after(ref);

}



/**

 * This method tests whether the date string passed in represents

 * a real date in mm/dd/YYYY format. 

 *

 * @param pDateString  date to be tested

 * @return a <code>boolean</code> value

 */

public static boolean isValidDate(String pDateString) {

    StringTokenizer st=  new StringTokenizer(pDateString.trim(), "/");

    if (st.countTokens() != 3) {

        throw new NumberFormatException("Date format should be MM/DD/YYYY.");

    }

    String month = st.nextToken();

    String day = st.nextToken();

    String year = st.nextToken();

    return toDate(year, month, day) != null; 

}



/**

 * Converts a String set of date values to a GregorianCalendar object.

 *

 * @param year a <code>String</code> value

 * @param month a <code>String</code> value

 * @param day a <code>String</code> value

 * @return a <code>GregorianCalendar</code> value

 */

public static GregorianCalendar toDate(final String year,

                                       final String month, 

                                       final String day) {



    int mm, dd, yyyy;



    try {

        if(year.length() != 4) {

            throw new NumberFormatException("Please provide four(4) digits for the year.");

        }

        yyyy = Integer.parseInt(year);

        if(yyyy == 0) {

            throw new NumberFormatException("zero is an invalid year.");

        }

     }

     catch(NumberFormatException nfe) {

         throw new NumberFormatException(year + " is an invalid year.");

     }



     try {

         mm = Integer.parseInt(month);

         if(mm < 1 || mm > 12) {

              throw new NumberFormatException(month + " is an invalid month.");

         }

     }

     catch(NumberFormatException nfe) {

         throw new NumberFormatException(month + " is an invalid month.");

     }



     try {

         dd = Integer.parseInt(day);

     }

     catch(NumberFormatException nfe) {

         throw new NumberFormatException(day + " is an invalid day.");

     }



     GregorianCalendar gc = new GregorianCalendar( yyyy, --mm, 1 );

     if(dd > gc.getActualMaximum(GregorianCalendar.DATE)) {

         throw new NumberFormatException(CalendarUtils.months[gc.get(GregorianCalendar.MONTH)] + " " + dd + " is an invalid day in "+gc.get(GregorianCalendar.YEAR) + ".");

     }

     if(dd < gc.getActualMinimum(GregorianCalendar.DATE)) {

         throw new NumberFormatException(CalendarUtils.months[gc.get(GregorianCalendar.MONTH)] + " " + dd + " is an invalid day in "+gc.get(GregorianCalendar.YEAR) + ".");

     }  

     return new GregorianCalendar(yyyy,mm,dd);

}

Correctness

My first point is that the current date is accepted as a valid future date. That is odd, and it is certainly not according to :

Tests whether the date input represents a real date in mm/dd/YYYY
format that is after the current date.

Simplicity

Secondly, assuming the current date should return false, the entire implementation can be as simple as :

public static boolean isValidDate(String pDateString) throws ParseException {

        Date date = new SimpleDateFormat("MM/dd/yyyy").parse(pDateString);

        return new Date().before(date);

}

Testable

Lastly, this method is difficult to test. Does it work when now falls in a DST overlap period? Does it work on 29th of February? Does it work in the time zone "Australia/Darwin"?

While you may not use Java 8, you can certainly make a Clock abstraction of your own to get around this.

try {

    if(year.length() != 4) {

        throw new NumberFormatException("Please provide four(4) digits for the year.");

    }

    yyyy = Integer.parseInt(year);

    if(yyyy == 0) {

        throw new NumberFormatException("zero is an invalid year.");

    }

 }

 catch(NumberFormatException nfe) {

     throw new NumberFormatException(year + " is an invalid year.");

 }

What happens if there is a non-numeric character in the year string? You could either accept a three digit year or reject a four digit year. E.g. -800 would be acceptable under this check but +2016 would not.

What happens if someone provides a five digit year? You'd throw an exception even though that would be a valid future date.

You catch anything that is a NumberFormatException but then throw another NumberFormatException with a less-specific message.

try {

    yyyy = Integer.parseInt(year);

    if ( yyyy <= 0 ) {

        throw new NumberFormatException("zero is an invalid year.");

    }

 }

 catch(NumberFormatException nfe) {

     throw new NumberFormatException(year + " is an invalid year.");

 }



 if ( yyyy < 100 ) {

     throw new NumberFormatException("Please provide all digits for the year (" + year + ").");

 }

This version accepts years with three or more digits but rejects two digit years (often abbreviations for four digit years). It throws a general invalid year exception if parseInt fails or if the year is non-positive. If the year is between 0 and 100, it assumes that it is abbreviated and asks for more digits.

This still catches and rethrows the exception on non-positive years, but it doesn't discard any information in doing so.

If you want to reject three digit years as well, you can just increase the boundary from 100 to 1000. However, I'm strongly against rejecting five or more digits, as that essentially puts an expiration date on your code. It may be early, but why introduce an unnecessary Y10K problem?

GregorianCalendar gc = new GregorianCalendar( yyyy, --mm, 1 );

Never do this. It's just confusing, use mm-1.

throw new NumberFormatException("zero is an invalid year.");

And negative is fine?

Your many catch-clauses provide a detailed message, but does anyone need it? A date isn't that complicated... a single catch clause stating "expected MM/DD/YYYY, got ..." should do.

If you really want to make it fast, then you surely shouldn't tokenize the string twice. And most probably you shouldn't write soooo much code.

You could also store the current midnight and start a ScheduledThreadPoolExecutor to update it regularly as this fooling around with the Calender can cost quite some time.

There's a SimpleDateFormat class doing about everything you need. But you don't need to convert anything to date for the comparison. Working with ints fully suffices. Working with the inputs chars would be most efficient, but a bit tedious.

To keep it simple:

// updated via the `ScheduledThreadPoolExecutor`

private int year;

private int month;

private int day;



private static final Pattern DATE_PATTERN =

    Pattern.compile("\s*(\d{1,2})/(\d{1,2})/(\d{4}))\s*");



public static boolean isValidDate(String date) {

    Matcher m = Pattern.matcher(date); // no trimming needed

    checkArgument(m.matches(), ...);

    int inputYear = Integer.parseInt(m.group(3));

    checkArgument(inputYear >= 1, ...);



    int inputMonth = Integer.parseInt(m.group(1));

    checkArgument(inputMonth >= 1 && inputMonth <= 12, ...);



    int inputDay = Integer.parseInt(m.group(2));

    checkArgument(inputDay >= 1 && inputDay <= daysInMonth(inputYear, inputMonth), ...);



    if (inputYear < year) return false;

    if (inputYear > year) return true;



    if (inputMonth < month) return false;

    if (inputMonth > month) return true;



    return inputDay > day;

}

I'm leaving daysInMonth as an exercise. :D You surely know that one-line conditionals are against the conventions (but I love them). checkArgument is from Guava.

As already said, solution reading the input char by char would be faster, but I'd go the opposite direction. Most probably, the speed is more than good enough, and the problem is called "stringly typed programming". Never use strings, when something better is available. Unfortunately, JDK Date is rather worse (worst class ever?), so if you don't want to use Joda, you should write your own. Immutable.This is rather trivial as long as you implement only what you really need and delegate to one the terrible JDK classes.