It's common to see methods returning sets of completely unrelated exceptions.  Many of these are thrown by inner functionality that the caller should not need to know about.  One philosophy of a method is to neatly contain logic in one place to make coding easier, not to spread the headache of inner workings to your callers.

Of course there are legitimate reasons why unrelated exceptions might be listed, but the following example illustrates the problem.

In the following method, three unrelated exceptions (which don't have the same base class) are passed back for the caller to handle, even though the caller may not know what to do with them.  The caller needs to handle many different types of exceptions just to use one method.  

protected void reprocess(File processFile, File targetFile)
    throws ProcessingException, RetryException, SkipStepException  
{
    File targetFile = new File(workFolder, processFile.getName());
    FileUtils.copyFile(processFile, targetFile);
    if (!processFile.delete())
    {
        // delete failed.
        throw new ProcessingException("Failed to delete " + processFile);
    }
    :
}

In this case, a RetryException meant “call me again straight away” so some extra management effort was offloaded to the caller when it could have been handled internally.

The method call would be easier to use if all exceptions were wrapped in one type.  If other types of exceptional conditions were possible, the subsequent exceptions could be subclasses of that type.

protected void reprocess(File processFile, File targetFile)  
    throws ProcessingException
{
    try
    {
        File targetFile = new File(workFolder, processFile.getName());
        FileUtils.copyFile(processFile, targetFile);
        if (!processFile.delete())
        {
            // delete failed.
            throw new ProcessingException("Failed to delete "  
                                         + processFile);
        }
        :
    }
    catch(IOException e)
    {
        // A serious file open problem.
        throw new AbortProcessingException(e);
    }
    catch(ProcessingException e)
    {
        // ProcessingException is already correct type.
        throw e;
    }
    catch(Exception e)
    {
        // Not a type we want to return, wrap it
        throw new ProcessingException(e);
    }
}

blog comments powered by Disqus