7

I am trying to parse dates using SimpleDateFormat. As my service takes in multiple date formats, I have adopted this approach:

String[] formats = {
        "yyyy-MM-dd'T'HH:mm:ss.SSSZ",
        "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'",
        "yyyy-MM-dd'T'HH:mm:ss.SSS-HH:mm",
        "EEE MMM dd HH:mm:ss Z yyyy"};

for (String format : formats)
{
    try
    {
        return new SimpleDateFormat(format).parse(dateString);
    }
    catch (ParseException e) {}
}
return null;

The rationale behind the try-catch is that if the current date format couldn't parse the dateString, an Exception would be thrown and the code would continue looping until a suitable date format is found, or return null.

The catch block is simply there so the try block would have something following it (a compile error could occur if nothing follows try).

I could leave the code just as it is, but empty catch blocks are bad practice. It would also be confusing for other people to maintain in the future. And it's simply inelegant.

I could put the following in the catch block :

catch (Exception e)
{
    if (!(e instanceof ParseException))
    {
        throw e;
    }
}

But again, the code inside serves no purpose as no Exception other that a ParseException could be thrown by the try block (I check for NullPointerException earlier in the code). Using a final block instead of a catch block would be useless also.

Is there a way to avoid the empty or useless catch block? Could try-catch be avoided completely?

Similar Questions (but not quite):

Avoiding an empty catch clause

Empty catch blocks

Is it ever ok to have an empty catch statement?

Community
  • 1
  • 1
SegFault
  • 2,526
  • 4
  • 21
  • 41
  • "no purpose as no Exception other that a ParseException could be thrown by the try block" there could be a `NullPointerException`, depending upon where `dateString` comes from. – Andy Turner Jul 19 '16 at 20:57
  • Sorry, I didn't include that in my code but there is a null pointer check before that. – SegFault Jul 19 '16 at 20:58
  • 2
    Your comment that "The catch block is simply there so the try block would have something following it" seems backward. If you didn't want either a `catch` or a `finally`, then you would not need or want a `try`, either. As far as I can determine, the `catch` block is there for the usual reason: you want to catch (certain) exceptions. – John Bollinger Jul 19 '16 at 21:03
  • "I could put the following in the catch block :" don't over-catch exceptions: all you need to handle is `ParseException`. Then, if a `RuntimeException` is thrown, that will be propagated; and the compiler guarantees that no checked exception except `ParseException` is thrown. So, that is just a complicated way of getting basically the same behaviour. – Andy Turner Jul 19 '16 at 21:07
  • I see. I originally had a logger in the catch block and I thought maybe I could get rid of the whole thing were no exceptions thrown anywhere. I will just keep a logger there. – SegFault Jul 19 '16 at 21:16

7 Answers7

3

Your code is fine. It is intentional and reasonable in this case to take no action when the SimpleDateFormat throws a ParseException. The only thing I would do differently is insert a documentary comment to that effect:

for (String format : formats)
{
    try
    {
        return new SimpleDateFormat(format).parse(dateString);
    }
    catch (ParseException e) {
        // The string does not conform to the trial format.
        // Just try the next format, if any.
    }
}

It is bad form to use empty catch blocks to avoid dealing with an exception that should be handled, instead. That's not what you're doing -- yours is the unusual case in which the correct handling is to do nothing.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
2

All answers given so far say that you have to live with exception catching, but there are ways to avoid exceptions at all. I demonstrate two ways, one with built-in SimpleDateFormat-API and one with using my library Time4J.

SimpleDateFormat

private static final List<SimpleDateFormat> SDF_FORMATS;

static {
    String[] formats =
        {
               "yyyy-MM-dd'T'HH:mm:ss.SSSX", 
               "yyyy-MM-dd'T'HH:mm:ss.SSS-HH:mm",
               "EEE MMM dd HH:mm:ss Z yyyy"
        };

    SDF_FORMATS = 
        Arrays.stream(formats)
            .map(pattern -> new SimpleDateFormat(pattern, Locale.ENGLISH))
            .collect(Collectors.toList());
}

public static java.util.Date parse(String input) {
  for (SimpleDateFormat sdf : SDF_FORMATS) {
    ParsePosition pos = new ParsePosition(0);
    java.util.Date d = sdf.parse(input, pos);
    if (pos.getErrorIndex() == -1) {
        return d;
    }
  }
  // log an error message
  return null; // or throw an exception
}

There is an observable performance improvement although not very spectacular compared with try-catch-code. One important caveat however, this presented code is NOT thread-safe. For usage in multi-thread-environment, you have to either always instantiate a new instance of SimpleDateFormat, or you can try to use ThreadLocal to minimize such instantiations.

Time4J

private static final MultiFormatParser<Moment> MULTI_FORMAT_PARSER;

static {
    String[] formats =
        {
               "yyyy-MM-dd'T'HH:mm:ss.SSSX", 
               "yyyy-MM-dd'T'HH:mm:ss.SSS-HH:mm",
               "EEE MMM dd HH:mm:ss Z yyyy"
        };

    List<ChronoFormatter<Moment>> formatters = 
        Arrays.stream(formats)
            .map(pattern -> 
                ChronoFormatter.ofMomentPattern(
                    pattern,
                    PatternType.CLDR,
                    Locale.ENGLISH,
                    Timezone.ofSystem().getID()))
            .collect(Collectors.toList());
    MULTI_FORMAT_PARSER = MultiFormatParser.of(formatters);
}

public static java.util.Date parse(String input) {
      ParseLog plog = new ParseLog();
      Moment m = MULTI_FORMAT_PARSER.parse(input, plog);
      if (plog.isError()) {
         // log an error message based on plog.getErrorMessage()
         return null; // or throw an exception
      } else {
         return TemporalType.JAVA_UTIL_DATE.from(m); // converted to old API
      }
}

This way is by far the quickest method to parse multiple formats. Try it out yourself (it is also possible to use Time4J on Java-6 or Android by using the version line 3.x but then you have to adjust the Java-8-streaming code in static initializer). The improvement in terms of performance is huge. And the code is also thread-safe.

General remark about your format patterns

  • I am worried about seeing the pattern "yyyy-MM-dd'T'HH:mm:ss.SSS-hh:mm" because "h" stands for 12-hour-clock (so AM/PM is missing!!!).
  • I am also worried about seeing the pattern "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'" because escaping the literal "Z" in input is wrong unless you explicitly set the GMT-Zone (zero offset) on your SimpleDateFormat-instance. Background: ISO-8601 defines such a pattern and always assigns the offset UTC+00:00 to the literal "Z". By escaping you will get results based on wrong calculations (without exception or warning).
Meno Hochschild
  • 42,708
  • 7
  • 104
  • 126
  • Thanks for the answer, I will explore the methods you mentioned. Also, I will change my date formats. I have to have the `'Z'` in one because I sometimes receive this kind of date: `2016-07-15T08:32:53.689Z`. – SegFault Jul 20 '16 at 15:27
  • @PhotometricStereo The ISO-text "2016-07-15T08:32:53.689Z" really means "2016-07-15T08:32:53.689+00:00" (zero offset from UTC). Therefore you must set `TimeZone.getTimeZone("GMT")` on your `SimpleDateFormat` for this case and not rely on your system timezone (which is the default). – Meno Hochschild Jul 20 '16 at 16:06
  • @PhotometricStereo If you want to use `SimpleDateFormat` and are on Java-7 then you can also use the pattern symbol "X" which can correctly interprete the literal "Z". – Meno Hochschild Jul 20 '16 at 16:09
1

If you want to avoid the try/catch block, it is possible, but it certainly increases the size of the code. The biggest issue I have with empty catch blocks that are uncommented is that they leave the question of "Did I mean to do //TODO: check for exceptions or not?" Code smell only really applies, IMO, if it makes no sense to do it that way, i.e. if you're parsing to see whether something is a number, rather than using an isNumber method.

You can create an explicit method designed to check whether it can be parsed, and then return the value if it is parseable.

boolean isParseable(Sting dateString, SimpleDateFormat format) {
    try {
        format.parse(dateString);
        return true;
    }
    catch(ParseException e) {
        return false;
    }
}

and then apply

for (String format : formats)
{
    SimpleDateFormat format = new SimpleDateFormat(format);
    if(isParseable(dateString, format))
        return format.parse(dateString);
}
return null;

Depending on how you want to handle the SDF object, you can choose to instantiate it twice, pass it around, or pass back null/String value.

Compass
  • 5,867
  • 4
  • 30
  • 42
1

When you're formatting a date by trying one format after another, it's reasonable you will have a bunch of exceptions, propagating them would be unhelpful, even seeing them is not helpful most of the time. The only change I would make would be to log the exception at DEBUG or TRACE level (set below where you'd have your normal logging when running the code in development), so just in case you wanted to come back and examine what was going on you'd only have to change the logging configuration. 

for (String format : formats)
{
    try
    {
        return new SimpleDateFormat(format).parse(dateString);
    }
    catch (ParseException e) {
        if (log.isTrace()) {
            log.trace("formatting failed for " + dateString 
            + " using format " + format, e);
        }
    }
}
return null;
Nathan Hughes
  • 94,330
  • 19
  • 181
  • 276
0

You can do this. But in my opinion (which is shared by a great deal of other professional software developers), empty catch blocks are an anti-pattern. You've encountered an exceptional case in your code; why ignore it? It means that you got bad/unusable input. That deserves some attention, wouldn't you agree?

So, yes, you can do this. But really, really ask yourself, do you want to? Or would you rather do something more intelligent with the bad input than silently throw it away. Let me give you this bit of advice: do yourself (and anyone else who will work with your code) the courtesy of at least logging the exceptional data. Don't just log the exception; try to capture what was wrong/exceptional with the state of your application in your log statement. In your case, I'd recommend logging what the failed date string was.

Finally, if you want to purely omit the catch-block... well, you can't outright ignore the exception. You can include a throws Exception declaration to your method prototype. But even then, anyone who uses your API will have to catch it. That's just part of how checked exceptions work in Java.

nasukkin
  • 2,460
  • 1
  • 12
  • 19
  • My question is asking how to avoid this, not asking whether it's ok or not. – SegFault Jul 19 '16 at 20:50
  • @PhotometricStereo check out the last paragraph of my answer. You can avoid writing a `catch`-block in your code, at the cost of expecting someone else to catch it. This is the only way. It is part of DateFormat's specifications to throw a checked exception (ParseException, specifically) if the string is unparseable. Language specifications prevent you simply ignoring them entirely. – nasukkin Jul 19 '16 at 20:52
  • You can't omit a catch block. A try has to be followed by at least a `catch` or a `final`. – SegFault Jul 19 '16 at 20:54
  • @PhotometricStereo That is true. I should have been a little bit more verbose, perhaps. When I say that you can omit the catch, I mean that you would omit the whole try-catch structure entirely. – nasukkin Jul 19 '16 at 20:55
0

You are saying you don't like the interface provided by SimpleDateFormat. Your only choice is to wrap that interface in another that provides the interface you are looking for. E.g.,

class MySimpleDateFormat {
  final String format;

  MySimpleDateFormat(format f) {
    this.format = f;
  }

  String format() {
    try {
      new SimpleDateFormat(format).format();
    } catch (Exception e) {
      return f.format();
    }
    return null;
  }
}
Jeffrey Blattman
  • 22,176
  • 9
  • 79
  • 134
0

I would use something like org.apache.commons.lang3.time.DateUtils.parseDate (or parseDateStrictly if you need to match the entire pattern). It takes a list of possible patters. Take a look to this example:

Date d = DateUtils.parseDate("23/10/2014T12:34:22", 
        new String[] {"yyyy/MM/dd'T'HH:mm:ss",
            "dd/MM/yyyy'T'HH:mm:ss"});

System.out.println(d);

Take a look to this doc: http://commons.apache.org/proper/commons-lang/javadocs/api-release/org/apache/commons/lang3/time/DateUtils.html

If none of the patterns apply to the input, you will get an exception. An empty catch block is a bad practice. You are suppose to deal with the exception, by either logging an error or throwing it or doing something else.

Perimosh
  • 2,304
  • 3
  • 20
  • 38