Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Avoid Empty Catch Blocks When Expecting Exception

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?

like image 417
SegFault Avatar asked Jul 19 '16 20:07

SegFault


People also ask

How do I stop empty catch blocks?

AVOID empty catch blocks. In general, empty catch blocks should be avoided. In cases where they are intended, a comment should be provided to explain why exceptions are being caught and suppressed. Alternatively, the exception identifier can be named with underscores (e.g., _ ) to indicate that we intend to skip it.

Is it OK to have an empty catch block?

Yes, we can have an empty catch block. But this is a bad practice to implement in Java. Generally, the try block has the code which is capable of producing exceptions, if anything wrong in the try block, for instance, divide by zero, file not found, etc. It will generate an exception that is caught by the catch block.

Can we handle exception without catch block?

Yes, it is possible. You can use an uncaught exception handler. Its responsibility is to catch the exceptions that your program didn't catch, and do something with it.

What happens if exception occurs in catch block?

Answer: When an exception is thrown in the catch block, then the program will stop the execution. In case the program has to continue, then there has to be a separate try-catch block to handle the exception raised in the catch block.


3 Answers

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).
like image 192
Meno Hochschild Avatar answered Oct 16 '22 18:10

Meno Hochschild


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.

like image 25
John Bollinger Avatar answered Oct 16 '22 18:10

John Bollinger


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.

like image 1
Compass Avatar answered Oct 16 '22 20:10

Compass