Use continue or Checked Exceptions when checking and processing objects

Question

I'm processing, let's say a list of "Document" objects. Before I record the processing of the document successful I first want to check a couple of things. Let's say, the file referring to the document should be present and something in the document should be present. Just two simple checks for the example but think about 8 more checks before I have successfully processed my document.

What would have your preference?

for (Document document : List<Document> documents) {
    if (!fileIsPresent(document)) {
        doSomethingWithThisResult("File is not present");
        continue;
    }
    if (!isSomethingInTheDocumentPresent(document)) {
        doSomethingWithThisResult("Something is not in the document");
        continue;
    }
    doSomethingWithTheSucces();
}

Or

for (Document document : List<Document> documents) {
    try {
        fileIsPresent(document);
        isSomethingInTheDocumentPresent(document);
        doSomethingWithTheSucces();
    } catch (ProcessingException e) {
        doSomethingWithTheExceptionalCase(e.getMessage());
    }
}

public boolean fileIsPresent(Document document) throws ProcessingException {
    ... throw new ProcessingException("File is not present");
}     

public boolean isSomethingInTheDocumentPresent(Document document) throws ProcessingException {
    ... throw new ProcessingException("Something is not in the document");
}

What is more readable. What is best? Is there even a better approach of doing this (maybe using a design pattern of some sort)?

As far as readability goes my preference currently is the Exception variant...

What is yours?

Answer 1

It depends more on how critical those two conditions are to the workflow and context you are dealing with.

If the "expected environment" is that the file exists and there is content in each document, then you would want to throw an exception because this is something out of the norm and needs to be dealt with in a special case.

If the "expected environment" is that files come and go, some have content, some don't, but the important thing is to get through the list of files being passed in, then having a graceful continue statement, and potentially logging or alerting of the bad data, would be preferable.

Answer 2

My preference is to only use exceptions when the situation is truly exceptional.

For example, it's natural for us as coders to assume that there is, in fact, a file for us to process. If the file isn't there, you have an exceptional and, perhaps, unrecoverable situation to deal with.

However, if there is something missing from the file, that might be less severe or more common. For example, if you are setting up a list of initial parameters and the user has failed to specify one of the parameters that you expect, you might be able to drop back to the default setting. That sort of behavior is considered a feature in our system since we expose some configuration scripts and properties files to our end users.

If you had thrown an exception, dropping out of the processing loop, you can't get back. However, if you only threw the exception within the loop to indicate "missing specification, using default", I don't think it would be worth the effort.

In short, I think it depends. In the specific example that you have above, I think I would end up using a combination of both styles.

Answer 3

Use continue, especially in loops. Most Exceptions create a stack trace that can be a major performance hit in a tight loop. You need to override fillInStackTrace to avoid the hit.

Stylistcally though, the general rule is that exception should be thrown in exceptional situations. If you are looking to see if a document exists and has certain data you should use continue unless you have a very high expectation that they all will have that data. But if the file not existing or not matching the formatting is anything but an unexpected occurrence, don't use exceptions.

Answer 4

It's not about readability per se, it's about safe programming. If you detected an error you should make sure the client code won't be able to ignore it unless it explicitly decided to do so (e.g. for retry).

Also, 'if' statements are much more vulnerable to accidental corruption of logic due to manual refactoring.

And, as you can see in your own example, error-reporting via exceptions are simply more readable.

P.S. Personally, I prefer to use unchecked exceptions. But it depends on the type of the code you do. Mine has no reason to continue if any exception happens, so I just promote it up to the very top start code, where I report it. Unchecked exceptions are less work in that case.

Answer 5

I think it would be more readable to refactor your checks.

for (Document document : List<Document> documents) {
    if(checkDocument(document)) {
        doSomethingWithTheSucces();
    } else {
        doSomethingWithTheError();
    }
}

// Later:
public boolean checkDocument(Document doc) { ... }

If you don't need to know WHY it failed then your checkDocument() can just return a boolean and maybe log what the problem is. If your code needs to know what the problem is, then something like this:

for (Document document : List<Document> documents) {
    try {
        checkDocument(document);
        doSomethingWithTheSucces();
    } catch(CheckDocumentException ex) {
        doSomethingWithTheError(ex);
    }
}

// Later:
public void checkDocument(Document doc) throws CheckDocumentException { ... }

You could also have checkDocument() return some kind of return code instead of an exception, but I think the exception is better for handling complex error data.

Both of these options removes the need for a continue or a large try{} block, and generally makes the code more readable. It also lets you reuse the logic for checking a document if you need to do so elsewhere later.