Is this additional check for parsing a string to LocalDate object necessary?

Question

I was writing some tests for some legacy code that validates a user's date of birth. I encounter the following method in the class. My doubt is that whether the if statement in the try block is necessary. From my understanding, if the parse function returns a LocalDate object successfully, then date.toString() should always equal to the input dobstr, and there's no need to do an additional check. Am I missing anything? I could not think of any case that we need this extra check. Please help. Thanks!

private LocalDate format(String dobStr) throws Exception {
        LocalDate date = null;
        try {
            date = LocalDate.parse(dobStr, DateTimeFormatter.ISO_DATE);
            if (!dobStr.equals(date.toString())) {
                 throw new DateTimeParseException("some message");
            }
        }
        catch (DateTimeParseException ex) {
            throw ex;
        }
        return date;
}

this is what I found in the source code for DateTimeFormatter.ISO_DATE

public static final DateTimeFormatter ISO_DATE;
static {
    ISO_DATE = new DateTimeFormatterBuilder()
            .parseCaseInsensitive()
            .append(ISO_LOCAL_DATE)
            .optionalStart()
            .appendOffsetId()
            .toFormatter(ResolverStyle.STRICT, IsoChronology.INSTANCE);
}

score 5 · Accepted Answer · answered Aug 11 '20 at 22:46

The only reason that I could see for doing a toString() check would be to avoid lenient issue: the parser may be lenient and try to interpret wrong values (for example: 2020-12-32 could be interpreted as 2021-01-01).

DateFormat allows the parser to be lenient
The same behaviour is offered by DateTimeFormatter and the default value is ResolverStyle.SMART.

If you want to remove it, you should check if DateTimeFormatter.ISO_DATE is ResolverStyle.STRICT by default or not. Assuming it is not STRICT by default, your code could be:

private LocalDate format(String dobStr) throws Exception {
  return LocalDate.parse(dobStr, DateTimeFormatter.ISO_DATE.withResolverStyle(ResolverStyle.STRICT));
}

Hi I edited the question. From the source code, I believe its default value is set to ResolverStyle.STRICT. I think it should be safe to remove that check. — LycheeSojuYYDS, Aug 11 '20 at 22:57

Ole V.V. · Answer 2 · 2020-08-12T18:53:44.840

TL;DR: The check makes a difference

If the string contains an unwanted offset ID, you will still be able to parse it using DateTimeFormatter.ISO_DATE. But since a LocalDate cannot have an offset (this is what local in the name means), the result of toString() will never have that offset ID, so the strings will not be equal.

DateTimeFormatter.ISO_DATE accepts an optional offset ID after the date. So if you are parsing 2020-08-12z or 2020-08-12+01:02:03, the custom exception would be thrown. Except for a detail: DateTimeParseException hasn’t got a constructor that matches a single string argument, so the code doesn’t compile. I reckon that this comes from sloppy copy-paste from the original code.

To demonstrate:

    String dobStr = "2020-08-12+01:02:03";
    LocalDate date = LocalDate.parse(dobStr, DateTimeFormatter.ISO_DATE);
    String asStringAgain = date.toString();
    System.out.format("Original string: %s; result of toString(): %s; equal? %s%n",
            dobStr, asStringAgain, dobStr.equals(asStringAgain));

Output is:

Original string: 2020-08-12+01:02:03; result of toString(): 2020-08-12; equal? false

How to obviate the check

Unless you require a custom exception in the case of an unwanted offset, the method may be written much more simply:

private LocalDate format(String dobStr) throws Exception {
    return LocalDate.parse(dobStr, DateTimeFormatter.ISO_LOCAL_DATE);
}

DateTimeFormatter.ISO_LOCAL_DATE does not accept any offset in the string. And it is strict just like DateTimeFormatter.ISO_DATE, so we know that toString() would create the same string again.

Furthermore you may declare the method static, and you may leave out throws Exception since DateTimeParseException is an unchecked exception.

Link

Documentation of DateTimeFormatter.ISO_DATE

Is this additional check for parsing a string to LocalDate object necessary?

2 Answers2

TL;DR: The check makes a difference

How to obviate the check

Link