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
t3chb0t
44.6k9 gold badges84 silver badges190 bronze badges
asked Jul 20, 2017 at 5:10
\$\endgroup\$

1 Answer 1

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.

answered Jul 20, 2017 at 6:18
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.