Pass null test then throws NullPointerException

Question

I am checking for Null in the code below, even when passed a bogus path the null test is passed. However when I return the workbook a NullPointerException is thrown. How can I check for a NullPointerReference in this case? When I inspect the workBook variable while debugging it is set to something, I believe this must be why it is passing the Null check.

public static ExcelWorkbook OpenExcelWorkSheet(string file_path)
{
    string EXCEL_FILE_EXTENSION = ".xlsx",
    full_path = file_path + EXCEL_FILE_EXTENSION;
    var excelFile = new FileInfo(full_path);
    using (var package = new ExcelPackage(excelFile))
    {
        ExcelWorkbook workBook = package.Workbook;
        if (workBook.Equals(null))
        {
            throw new Exception("ERROR!!!");
        }
        else { Console.Write("not null"); }
        return workBook;

    }
}

Cᴏʀʏ · Accepted Answer · 2014-06-12T22:13:15.723

Looking at the source code for ExcelPackage, the Workbook property can never return null. Its getter will create a new Workbook if it finds that its internal field is null:

public ExcelWorkbook Workbook
{
    get
    {
        if (_workbook == null)
        {
            var nsm = CreateDefaultNSM();

            _workbook = new ExcelWorkbook(this, nsm);

            _workbook.GetExternalReferences();
            _workbook.GetDefinedNames();

        }
        return (_workbook);
    }
}

Knowing this, I think you could safely remove the check altogether.

Additionally, as others have noted, you do need to be careful about how you use your using statements. When a ExcelPackage is disposed, its Workbook is in turn disposed (you can see this happening in the source code I linked). This is probably why you're seeing a null workbook returned from your method, but not while you are in the using statement. Very shortly after that closing } of your using it becomes null.

If you have a class that you're using to wrap the functionality of EPPlus, then you should make sure that it:

Only instantiates the ExcelPackage once and holds onto a reference to it; do not use a using statement when you do this
Have your other methods access this ExcelPackage reference when they need to; don't pass it around or re-open the package
Implement IDisposable, then properly dispose your ExcelPackage object when your wrapper is disposing.

Here's an example*:

public class YourWrapper : IDisposable
{
    private bool disposed;

    public YourWrapper()
    {
    }

    public YourWrapper(string path)
    {
        OpenExcelPackage(path);
    }

    public void OpenExcelPackage(string path)
    {
        Package = new ExcelPackage(path);
    }

    public ExcelPackage Package { get; private set; }

    protected virtual void Dispose(bool disposing)
    {
        if (!this.disposed)
        {
            if (disposing)
            {
                if (Package != null)
                {
                    Package.Dispose();
                    Package = null;
                }
            }
        }
        this.disposed = true;
    }

    ~YourWrapper()
    {
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

Now instead, use a using whenever you instantiate your wrapper:

using (var wrapper = new YourWrapper())
{
    // Do everything related to manipulating the Excel file here
}

And everything will get cleaned up nicely.

^{* Please change the names of the classes/methods to something more appropriate; this was just for illustration.}

Pass null test then throws NullPointerException

1 Answers1