2
\$\begingroup\$

I've used this code to get the sum of the values of each column and display in footer. Here, the advantage is that I don't have to look for the column name, just get the count of total number of rows and columns and then iterate over each row and column.

Please suggest how I can make this code more efficient or if there is any way to make this code block more effective.

#region column sum and show in footer
//loop all the rows in the datatable
int[] RowTotals = new int[dt_.Columns.Count];
foreach (DataRow row in dt_.Rows)
{
    if(dt_.Rows.IndexOf(row) !=0)
    {
        //loop all the columns in the row
        for (int i = 1; i < dt_.Columns.Count; i++)
        {
            //add the values for each cell to the total

            RowTotals[i] += Convert.ToInt32(row[dt_.Columns[i].ColumnName]);
        }
    }

}

//loop all the columns again to set the values in the footer

for (int i = 1; i < dt_.Columns.Count; i++)
{

    GridView1.FooterRow.Cells[i].Text = string.Format("{0:N2}", RowTotals[i]);
}
///////
#endregion
\$\endgroup\$
2
\$\begingroup\$
#region column sum and show in footer

Before you start optimizing this block you should move it into a separate method. The region clearly indicates that it's a part of something bigger that is doing too much. I find it's pointless to try to optimize this as long as it's a region of something else.

Currently your method not only calculates the RowTotals but it also updates a grid-view. This should not be done by the same method because you cannot test whether the totals are calculated correctly. The only way to validate it is to look at the grid. This is very unreliable.

Basically it should look like this: you have two worker methods

int[] CalculateRowTotals(DataTable dataTable) { ... }

void UpdateTotalsGridView(int[] rowTotals) { ... }

and one method that combined the results of both:

void DisplayRowTotals(..)
{
    var rowTotals = CalcualteRowTotals(..);
    UpdateTotalsGridView(rowTotals);
}

About the code itself...

if(dt_.Rows.IndexOf(row) !=0)

It is not clear why you are doing this. A helper variable or method would be nice so that you can give it a meaning like I don't know isHeader or something that makes sense.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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