Assigning values to a particular table column

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

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.

StackExchange Code Review Q#84564, answer score: 3