4
\$\begingroup\$

I have a set of Excel data which I read and assign to corresponding columns of an Excel table. In our Excel we have provided the user the option to rearrange the Excel column fields at runtime.

int rowIndex =int.Parse(ExcelSettingManager.ItemLine);

int lastItem = numberOfFilledRows();

numberOfFilledRows();

int maxIndex = rowIndex + lastItem;

MSExcel.Range cell = null;
while (rowIndex < maxIndex)
{
    DataRow dataRow = ExcelDataManager.Instance.DocumentItemTable.NewRow();
    cell = this.worksheet.Range[ExcelSettingManager.LineItemNo + rowIndex.ToString()];
    if (cell.Value2 == null)
    {
        return;
    }


    dataRow["LineItemNo"] = cell.Value;
    ExcelUtilities.ReleaseResources(cell);

    if (ExcelSettingManager.CostCenter != string.Empty)
    {
        cell = this.worksheet.Range[ExcelSettingManager.CostCenter + rowIndex.ToString()];
        dataRow["CostCenter"] = cell.Value;
        ExcelUtilities.ReleaseResources(cell);
    }

    if (ExcelSettingManager.GLAccount != string.Empty)
    {
        cell = this.worksheet.Range[ExcelSettingManager.GLAccount + rowIndex.ToString()];
        dataRow["GeneralLedgerAccount"] = cell.Value;
        ExcelUtilities.ReleaseResources(cell);
    }

    if (ExcelSettingManager.Description != string.Empty)
    {
        cell = this.worksheet.Range[ExcelSettingManager.Description + rowIndex.ToString()];
        dataRow["ItemText"] = cell.Value;
        ExcelUtilities.ReleaseResources(cell);
    }
}

internal int numberOfFilledRows()
{
    int noft = 0;
    string sNextCell = ExcelSettingManager.LineItemNo ExcelSettingManager.ItemLine;
    MSExcel.Range cell = this.worksheet.Range[sNextCell];

    while(!string.IsNullOrEmpty(cell.Value))
    {
        ExcelUtilities.ReleaseResources(cell);
        noft++;
        sNextCell = ExcelSettingManager.LineItemNo + (int.Parse(ExcelSettingManager.ItemLine) + noft).ToString();
        cell = this.worksheet.Range[sNextCell];
    }
    return noft;
}

If my Excel has 10 rows according to the current logic, it will read cell by cell each row and will assign to the corresponding Excel table column.

This is having a lot of performance impact. When there are more rows, the performance is worse.

How can I increase performance of this code?

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Have you tried anything to see what lines are causing the slowdown? Is it the .NewRow(), the .ReleaseResources(), the string concatenation, the use of .Range() instead of specifying the cell row and column by using integers? Even if you don't want to use a profiler (you should), there are ways of determining if certain lines are a speed hit. \$\endgroup\$
    – Snowbody
    Commented Mar 20, 2015 at 16:04

1 Answer 1

3
\$\begingroup\$

One big problem is that your code is not complete so I have to guess. In the future please post complete working code.

There are some obvious inefficiencies in your code, though I'm not sure if the compiler will fix them for you or not, but it's still good programming practice to:

  • What's the point of the second numberOfFilledRows(); ?
  • You have the same code repeated three times, with the only difference being the two strings, one used as the index to dataRow[] and the other being put in the string concatenation. In cases like this you should factor out the common code to a separate function.
  • You repeatedly call rowIndex.ToString(). The compiler may be smart enough to put this in a temporary variable, but this is a clue that you're doing something the wrong way. You don't say what type worksheet is, but isn't there a method that takes two ints for the row and column numbers?
  • your numberOfFilledRows() is reinventing the wheel. Excel already has functions to find the end of a range (probably much faster too).

EDIT Also, the fastest way to check if a string is empty is to compare the .Length member to 0. See http://www.dotnetperls.com/empty-string

You may also want to look into StyleCop and/or FXCop.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.