Exceptions handling, what would you do?

Question

I know that there are several ways to handle exceptions and probably a good way is to keep the handling consistently and use the pattern which feels comfortable for himself, but I would like to get some advice or hints, what would you do in following situation or how I could improve such a case.

I created a custom URLClassLoader which checks the certification and some advanced stuff. With following function I load the X509 certificate into the instance:

public boolean loadX509Certificate(File file) {
    try {
        CertificateFactory cf = CertificateFactory.getInstance("X.509");
        InputStream is = new FileInputStream(file);
        this.setCertificate(cf.generateCertificate(is));
        is.close();
    } catch (CertificateException e) {
        return false;
    } catch (FileNotFoundException e) {
        return false;
    } catch (SecurityException e) {
        return false;
    } catch (IOException e) {
        return false;
    }
    return true;
}

In that function I catch four exceptions, what would you do in this situation? Return false if the certificate cannot load, rethrow the exception or create a custom exception which tells the user what went wrong?

In generally the Class will work without the certification, but if the value doesn't exist, the class will not check the classes and resources signature.

Answer 1

First, I would separate loading the certificate from storing it.

• They are independent concerns.
• It allows multiple ways to load a certificate.
• It improves testability.

As Kinjal suggests, logging the error is a good alternative when failure is recoverable. However, I would throw a custom exception (or use CertificateException) and let the caller decide to log and continue or terminate. I really don't like returning success codes when I can avoid it because it muddies the code with checks.

Finally, instead of setting a certificate or leaving the field blank consider modeling a certificate checking strategy. This would require a simple one-method interface CertificateCheck with two implementations: NullCertificateCheck and BasicCertificateCheck. Again this improves testability by providing a seam for mocking, separating concerns, etc. It also removes the need for if (this.certificate != null) checks and allows for more strategies and certificate types.

public void setupCertificate() {
    try {
        certificateCheck = loadX590Certificate(...);
    }
    catch (InvalidCertificateException e) {
        LOG.warn("Skipping certificate check", e);
        certificateCheck = new NullCertificateCheck();
    }
}

private CertificateCheck loadX509Certificate(File file) throws CertificateException {
    InputStream is = null;
    try {
        CertificateFactory cf = CertificateFactory.getInstance("X.509");
        is = new FileInputStream(file);
        return new BasicCertificateCheck(cf.generateCertificate(is));
    }
    catch (FileNotFoundException e) {
        throw new CertificateException("Certificate file not found", e);
    }
    ... more exceptions ...
    finally {
        IOUtils.closeQuietly(is);
    }
}

Update: Since you're storing the certificate (or not) so you can check its validity, I assumed you were checking it at a later time and ideally multiple times. If this is the case, extract the logic into a CertificateCheck hierarchy using the Strategy pattern. If the certificate is checked once in isolation of any other data, this isn't necessary and I see no reason to store the certificate in an instance field.

Note: I'm using Checkable as a stand-in for whatever contains the information that must be checked against the certificate. Your implementation would need to pass whatever is appropriate.

public interface CertificateCheck {
    boolean matches(Checkable thing);
}

// in the absence of a certificate, everything matches
public class NullCertificateCheck implements CertificateCheck {
    public boolean matches(Checkable thing) {
        return true;
    }
}

public class BasicCertificateCheck implements CertificateCheck {
    private final Certificate cert;

    public BasicCertificateCheck(Certificate cert) {
        this.cert = cert;
    }

    public boolean matches(Checkable thing) {
        // move the current checks you have here
    }
}

Answer 2

For Java 7 source code type You could use multicatch. http://www.baptiste-wicht.com/2010/05/better-exception-handling-in-java-7-multicatch-and-final-rethrow/

Personally I would log this error and return false.

Answer 3

It depends on your decision if you want to move further when certification fails. You can throw exception if certification is "must" and you do not want to proceed if there is any issue with it.

In other case, if you still want the execution to move further when there is any problem with certification then just logging the message would be fine.

Here I have modified your code --

public boolean loadX509Certificate(File file) {
  InputStream is = null;
  boolean isCertificateLoaded = false;

  try {
    CertificateFactory cf = CertificateFactory.getInstance("X.509");
    is = new FileInputStream(file);

    this.setCertificate(cf.generateCertificate(is));

    isCertificateLoaded = true;
  } catch (CertificateException e) {
    // log certification error 
  } catch (FileNotFoundException e) {
    // log file not found error 
  } catch (SecurityException e) {
    // log security error 
  } catch (IOException e) {
    // log i/o error 
  } finally {
    if (null != is) { is.close(); }
  }

  return isCertificateLoaded;
}