Color Rows In DataGridView

Question

I am using the below syntax to color code sales persons data. The issue that I have is that the datagridview returns roughly 12,000 rows so the coloring syntax takes some extensive time to execute.

Is there a way to optimize this code?

private void datagridview1_CellFormatting(object sender, DataGridViewCellFormattingEventArgs e)
{
    foreach (DataGridViewRow row in datagridview1.Rows)
    {
        if (row.Cells["Jake"].Value == System.DBNull.Value && row.Cells["Jones"].Value == System.DBNull.Value && row.Cells["Lati"].Value == System.DBNull.Value && row.Cells["Venitia"].Value == System.DBNull.Value)
        {
            row.DefaultCellStyle.BackColor = Color.Red;
        }
    }            
}
private void PopulateIT()
{
    datagridview1.Visible = true;
    lblNoData.Visible = false;

    string connString = @"Connection String";
    string query = @"SELECT * from tempdata";
    SqlConnection conn = new SqlConnection(connString);
    SqlCommand cmd = new SqlCommand(query, conn);
    conn.Open();
    SqlDataAdapter da = new SqlDataAdapter(cmd);
    da.Fill(formattedDT);
    conn.Close();
    da.Dispose();
    datagridview1.DataSource = formattedDT;
    datagridview1.AutoResizeColumns();
}

Answer 1

SqlConnection, SqlCommand and SqlDataAdapter are implementing the IDisposable interface. Enclosing the usage of these objects inside a using statement ensures that the objects will always be proper disposed because the using statement just equals a try...finally where the objects would be disposed in the finally part.

The main problem regarding your performance issue is, that by calling the AutoResizeColumns() method the event will be triggered for each row and each column.
This results for each row and each column that you iterate over every row in the DataGridView.

So if we assume you have 12,000 rows with at least 5 columns .... you can do the math yourself.

The DataGridViewCellFormattingEventArgs has a property RowIndex which allows to access a single row so you should use this.

Summing this up in some code could look like so

private bool shouldFormattingBeDone = false;
private void PopulateIT()
{
    datagridview1.Visible = true;
    lblNoData.Visible = false;

    string connString = @"Connection String";
    string query = @"SELECT * from tempdata";

    using (SqlConnection conn = new SqlConnection(connString))
    using (SqlCommand cmd = new SqlCommand(query, conn))
    using (SqlDataAdapter da = new SqlDataAdapter(cmd))
    {
        conn.Open();
        da.Fill(formattedDT);
    }
    datagridview1.DataSource = formattedDT;

    shouldFormattingBeDone = false;
    datagridview1.AutoResizeColumns();
    shouldFormattingBeDone = true;
}


private void datagridview1_CellFormatting(object sender, DataGridViewCellFormattingEventArgs e)
{
    if (shouldFormattingBeDone == false) { return; }

    DataGridViewRow row = datagridview1.Rows[e.RowIndex];

    if (row.Cells["Jake"].Value == System.DBNull.Value && row.Cells["Jones"].Value == System.DBNull.Value && row.Cells["Lati"].Value == System.DBNull.Value && row.Cells["Venitia"].Value == System.DBNull.Value)
    {
        row.DefaultCellStyle.BackColor = Color.Red;
    }

}

This could be beautified by having an extension method which takes a DataGridViewCell as a parameter like so

public static class DataGridViewExtension
{
    public static bool IsDbNull(this DataGridViewCell cell)
    {
        return cell.Value == DBNull.Value;
    }
}  

resulting in the if condition looking like so

if (row.Cells["Jake"].IsDbNull() && row.Cells["Jones"].IsDbNull() && row.Cells["Lati"].IsDbNull() && row.Cells["Venitia"].IsDbNull())  

PS: I can't remember if the connection needs to be open when the constructor of the SqlDataAdapter is called.

Answer 2

I don't like that repeated code pattern in the if...DbNull &&... . I think the field names should be stored in an array/list and then you could do something LINQy like

if (fieldNamesList.All(fieldName=>row.Cells[fieldName].IsDbNull()))

That way it would be easier to add and remove field names without having to make error prone copy and paste, also the line is shorter.

Also I hope that in your actual code, instead of this stripped-down version, the connection string is read from a config store instead of being hardcoded.