HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Reading from an Excel sheet

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromreadingsheetexcel

Problem

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; 
    }

Solution

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();
    }
}

Code Snippets

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();
    }
}

Context

StackExchange Code Review Q#14756, answer score: 3

Revisions (0)

No revisions yet.