No, leaving the caller to handle the Exception is fine - throw early catch late.

What I have a problem with is this:

}catch (DocumentException e){
    throw new DocumentException("some message");


Why would you catch (DocumentException e) and then throw a new instance that strips out all useful information? You can simply not catch it in the first place and let it percolate up to someone who can handle it.

Also, use Java 7 try-with-resources rather than finally. So, you code should be:

public static String readFile(String xmlFileName) throws IOException, DocumentException {
    try (final InputStream is = new ClassPathResource(xmlFileName).getInputStream()) {
        final SAXReader reader = new SAXReader();
        final Document doc = reader.read(inputStream);
        return doc.asXML();
    }
}


I have removed the variables that are declared as null then reassigned, I hate this practice and so do many other Java developers - get out of this habit. Declare things when you need them and assign them immediately. In a garbage collected language the principal of minimum scope is very important.

I have also changed it to return directly rather than storing the value for some reason.

Is catching DocumentException a good idea?
I can't figure out why it's looking for something static:

public class DatabaseBase
{
    private readonly string connectionString;

    public DatabaseBase(string connectionString)
    {
        this.connectionString = connectionString;
    }
}

public class MyDB : DatabaseBase
{
    readonly string connectionString = ConfigurationManager.AppSettings["MyConnectionString"];

    public MyDB() : base(connectionString)
    {          
    }
}


I get Cannot access non-static field 'connectionString' in static context.  I don't see anything static in the base Database class so why??

here's another example of when we did kinda the same thing:

partial class Database : DatabaseBase
{
    static string DbConnectionString
    {
        get
        {
            if (dbConnectionString == null)
                dbConnectionString = 
                    ConfigurationManager.AppSettings["MyConnectionString"];
            return dbConnectionString;
        }
    }
    public Database() :base(DbConnectionString)
    {
    }


ok so why did it have to be a static string for the connection string to be passed?
