Search code examples
datagridviewnullreferenceexceptionhighlight

How do I avoid datagridrow highlight function bombing out on Null?


I have a function whcih is designed to highlight rows where certain cells don't match each other. This works until it hits a cell which contains nothing, then it bombs out as a nullrefexception. I know I'm missing something simple here but I'm banging my head against a wall trying to figure it out! Thanks in advance.

        private void highlightrow()
    {
        int rowcnt = 0;
        dataGridView1.ClearSelection();
        int deviceNameColIndex = dataGridView1.Columns["cDeviceName"].Index;
        int deviceNameColIndex2 = dataGridView1.Columns["cDeviceName2"].Index;
        int driverVerColIndex = dataGridView1.Columns["cDriverVersion"].Index;
        int driverVerColIndex2 = dataGridView1.Columns["cDriverVersion2"].Index;
        int driverProviderName = dataGridView1.Columns["cdriverProviderName"].Index;
        int driverProviderName2 = dataGridView1.Columns["cdriverProviderName2"].Index;

        try
        {
            foreach (DataGridViewRow row in dataGridView1.Rows)
            {

                rowcnt++;

                if (row.Cells[driverVerColIndex].Value.ToString() != row.Cells[driverVerColIndex2].Value.ToString() ||
                    row.Cells[driverProviderName].Value.ToString() != row.Cells[driverProviderName2].Value.ToString() ||
                    row.Cells[deviceNameColIndex].Value.ToString() != row.Cells[deviceNameColIndex2].Value.ToString()
                    )
                {
                    dataGridView1.DefaultCellStyle.SelectionBackColor = Color.YellowGreen;
                    row.Selected = true;
                }

            }
        }

        catch (NullReferenceException) { }
 }

How do I skip null cells?


Solution

  • I meant no offense with my comment. I was just trying to nudge you to a better solution. Whether or not you choose to attempt to find a better solution is up to you. In defense of my comment, specifically in reference to the undesired side effects the foreach loop through the columns was creating... I was simply trying to point out “why” that approach would create some undesired/unexpected results. Please forgive me if it came across as something other than a helpful comment.

    Using the current posted example, below is one possible “better” approach when trying to avoid the null reference exception specifically when looping through a DataGridView.

    For starters, when looping through the rows in a grid and checking cell values, there is “always” the possibility that a cells value may be null. In some cases you “may” assume that the cells value is not null, however, even in those rare cases, as a coder, if the cell is “somehow” null and the code does not check for this… then the code crashes. The consequences are too high to ignore this check.

    Therefore, when looping through the grid’s cells... it is wise to check for these null values before we try to call its ToString method.

    One place to check for null values is if the row is the grids “new” row. If the grid’s AllowUsersToAddRows property is set to true AND the grids data source allows new rows, then you are guaranteed that the cells in that row are null. Therefore, when looping through the rows, checking for this new row will eliminate the need to check the cells. We know they are all null so we can “skip” that row.

    You may consider simply changing the grids AllowUserToAddRows property to false and skip this check, however, what if at a later date, that property gets changed to true? If we “always” check for this “new” row, whether it exist or not, will allow the code to work regardless of what the AllowUserToAddRows property is to.

    So, the first check in the foreach loop through the rows would be this check for the new row…

    foreach (DataGridViewRow row in dataGridView1.Rows) {
      if (!row.IsNewRow) { … //this row it NOT the new row … }
    

    This eliminates the “new” row; however, we still need to check each cell. In this case, the code would have to check the six (6) Cell.Value’s for null THEN the code would then need to compare the six (6) values two at a time. Using a bunch of if statements could get messy.

    So, it may help if we create a method that checks two cell values and returns true if the cell values are equal and false if they are not equal. This should reduce the number of if statements we need. This method may look something like…

    private bool CellsAreEqual(DataGridViewCell cell1, DataGridViewCell cell2) {
      if (cell1.Value == null && cell2.Value == null) {
        return false;
      }
      if (cell1.Value != null && cell2.Value != null) {
        return cell1.Value.ToString().Equals(cell2.Value.ToString());
      }
      else {
        // one cell is null the other cell is not null
        return false;
      }
    

    The codes logic assumes that if one or both cells are null, then, the cells are NOT equal. First a check is made to see if both cells are null and if so, returns false. If one cell is null and the other cell is not null, then return false, otherwise we can safely call each Cell.Value.ToString method.

    Finally, with the above method and the new row info, we can change the highlightrow method to something like…

    private void highlightrow() {
      dataGridView1.ClearSelection();
      foreach (DataGridViewRow row in dataGridView1.Rows) {
        if (!row.IsNewRow) {
          if (!CellsAreEqual(row.Cells["cDriverVersion"], row.Cells["cDriverVersion2"]) ||
              !CellsAreEqual(row.Cells["cdriverProviderName"], row.Cells["cdriverProviderName2"]) ||
              !CellsAreEqual(row.Cells["cDeviceName"], row.Cells["cDeviceName2"])) {
            dataGridView1.DefaultCellStyle.SelectionBackColor = Color.YellowGreen;
            row.Selected = true;
          }
        }
        else {
          // ignore new row
        }
      }
    }
    

    I hope this makes sense.