Error handling for parsing a date

Question

Is this return style acceptable? Would the code be more readable if I assigned the MonthDay.parse() return value to a variable and returned that variable on the last line?

Also, is it a bad idea to both log and rethrow inside a catch block?

/**
 * Parses showing date from the header text.
 * 
 * @throws PageStructureException if the date string does not conform to the expected format
 */
public MonthDay parseShowingDate( String date ) throws PageStructureException {
    Preconditions.checkNotNull( date );

    DateTimeFormatter matDTF = DateTimeFormat.forPattern( DATE_FORMAT );
    String dayAndMonth = removeDayOfWeek( date );
    try {
        return MonthDay.parse( dayAndMonth, matDTF );
    } catch ( IllegalArgumentException iae ) {
        logger.error( "Could not parse MAT date {}, expected format [{}].", date, DATE_FORMAT );
        throw new PageStructureException( iae );
    }
}

Answer 1

I believe it's OK. This way, it is immediately clear from the code that the method returns a value only when no exception is thrown during parsing. If you used a variable to store the result, it becomes less clear. And, the reader has to track where the variable is initialized/modified/returned.

Logging an exception when it's rethrown is a bit confusing, because it is possible the caller expect the possibility of receiving an exception and deals with it. But sometimes a programmer needs it so that it's possible to examine what's happening even if the caller catches the exception. In such a case, I'd suggest using only debug or trace logging levels.

Answer 2

It is a very good understandable write.

You can also write the return method in a parameters, it is optimal (look in Effective Java all subtle reasons):

try {
    return MonthDay.parse( removeDayOfWeek(date), DateTimeFormat.forPattern(DATE_FORMAT));
} catch ( IllegalArgumentException iae ) {
    logger.error( "Could not parse MAT date {}, expected format [{}].", date, DATE_FORMAT );
    throw new PageStructureException( iae );
}

I've not included here Preconditions.checkNotNull( date ), because it is not really a parameters, but I'll do it if it was my job,