Reading from an Excel sheet

Question

I have created a class for reading data from an Excel sheet and holding on the variable. I am not sure if my code is optimized, meaning I will be reading heavily from Excel.

Is there a room to further optimized this code? I will be using this class heaving in my code, and I wanted to get some opinions/feedback.

I will be calling something like this:

ExcelReader xl = new ExcelReader();
string sss = xl.ExcelOpenSpreadsheets("mysheet");
//etc...

public class ExcelReader  
{             
    Application _excelApp;

    public ExcelReader()
    {
        _excelApp = new Application();
    }

    public string ExcelOpenSpreadsheets(string sheetName)
    {
        string _txt = string.Empty;
        try
        {
            Workbook workBook = _excelApp.Workbooks.Open("filename_here",....);
            _txt = ExcelScanIntenal(workBook, sheetName);
        }

        catch
        {
            //
            // Deal with exceptions.
            //
        }
        return _txt;
    }

    private string ExcelScanIntenal(Workbook workBookIn, string sheetName)
    {
        Worksheet sheet = workBookIn.Sheets[sheetName] as Worksheet;

        Range a1 = sheet.get_Range("A1", "B2");
        if (a1 != null)
        {
            string formattedText = r.Text;                   
        }

        return formattedText; 
    }

Answer 1

I'd change the code so that every implicit constant value is an argument in the constructor, with default values.

In addition, I guess this excel application is the COM API for Excel, so perhaps a better approach would be using the IDisposable interface and implementation.

I changed the typo and now the inner method's name is ExcelScanInternal (and not ExcelScanIntenal).

So the result looks like this (I didn't compile it, so it might have syntax errors):

public class MyRange
{
    public string @From { get; set; }
    public string @To { get; set; }
}

public class ExcelReader : IDisposable
{             
    Application _excelApp;
    string _fileName;
    MyRange _scanRange;

    public ExcelReader() : this ("the_file_name", new MyRange{From = "A1", To = "B2"})
    {
    }

    public ExcelReader(string fileName, MyRange scanRange)
    {
        if (null == fileName) throw new ArgumentNullException("fileName");
        _fileName = fileName;
        if (null == scanRange) throw new ArgumentNullException("scanRange");
        _scanRange = scanRange;
        _excelApp = new Application();        
    }

    public string ExcelOpenSpreadsheets(string sheetName)
    {
        string _txt = string.Empty;
        try
        {
            Workbook workBook = _excelApp.Workbooks.Open(_fileName,....);
            _txt = ExcelScanInternal(workBook, sheetName);
        }

        catch
        {
            //
            // Deal with exceptions.
            //
        }
        return _txt;
    }

    private string ExcelScanInternal(Workbook workBookIn, string sheetName)
    {
        Worksheet sheet = workBookIn.Sheets[sheetName] as Worksheet;

        Range a1 = sheet.get_Range(_scanRange.From, _scanRange.To);
        if (a1 != null)
        {
            string formattedText = a1.Text;                   
        }

        return formattedText; 
    }

    public void Dispose()
    {
        _excelApp.Quit();
    }
}

Answer 2

Variable names make sense, except the r one.

Good use of white space and tabs.

I'm not sure if this can be optimized any more, but I do notice this method will not work

private string ExcelScanIntenal(Workbook workBookIn, string sheetName)
{
    Worksheet sheet = workBookIn.Sheets[sheetName] as Worksheet;

    Range a1 = sheet.get_Range("A1", "B2");
    if (a1 != null)
    {
        string formattedText = r.Text;                   
    }

    return formattedText; 
}

formattedText is declared within your if statement which may or may not be true.

You have a random variable r show up in the middle of your function, I'm assuming it should be a1.

I'd change it to this:

private string ExcelScanIntenal(Workbook workBookIn, string sheetName)
{
    Worksheet sheet = workBookIn.Sheets[sheetName] as Worksheet;

    Range a1 = sheet.get_Range("A1", "B2");
    if (a1 != null)
    {
        return a1.Text;                   
    }

    return string.Empty; 
}

Answer 3

Seems to me that caching will help to the extent that you avoid reading from disk. If "20% of the sheets get read 80% of the time" - i.e. some sheets get read many times.

System.Web.Caching has a Cache class. Even though this is in the Web namespace it will work. Here is a quote from a StackOverflow Posting

How are you implementing your cache?

You can use the Cache class from System.Web.Caching, even in non-web applications, and it will purge items on an LRU basis if/when it needs the memory.

In a non-web application you'll need to use HttpRuntime.Cache to access the Cache instance.

Note that the documentation states that the Cache class isn't intended to be used outside of ASP.NET, although it's always worked for me. (I've never relied on it in any mission-critical app though.)

P.S. "LRU" means Least Recently Used. It's an algorithm for deciding what to discard to make room as the cache fills up.