0

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?

  • For any cell in the grid, you should check to make sure the cells `Value` is NOT `null` BEFORE you try and call its `ToString` method. All the lines…`row.Cells[xxx].Value.ToString()` … assume the cells `Value` is not `null` and apparently one or more is `null` and is throwing the null reference exception. You should “check” to make sure `Value` is NOT `null` like… `if (row.Cells[driverVerColIndex].Value != null) {…}`…BEFORE you call it’s `ToString` method. Example, If the grids `AllowUsersToAddRows` property is `true`… then the cells in the “new” row will be `null.` – JohnG Jan 20 '21 at 16:08
  • 1
    Does this answer your question? [What is a NullReferenceException, and how do I fix it?](https://stackoverflow.com/questions/4660142/what-is-a-nullreferenceexception-and-how-do-i-fix-it) – JohnG Jan 20 '21 at 16:11

2 Answers2

0

Thanks for the ideas. I have tried checking for null before but was still having issues so was looking for some practical examples. As it happens, I have managed to sort it, though still feel this could be done better:

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;

    foreach (DataGridViewRow row in dataGridView1.Rows)
    {
        rowcnt++;

        foreach (DataGridViewCell cell in row.Cells)
        {
            string val = cell.Value as string;
            if (String.IsNullOrEmpty(val))
            { }
            else
            {
                try
                {
                    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 { }
            }
        }

    }
}
  • The added loop through the columns may well catch a cell’s `null` value, however, if the cells are not `null` then the code will execute the `if` statement comparing the different cells at least six (6) times for every row when it only needs to execute once for each row? Are you sure this is a good solution? – JohnG Jan 20 '21 at 18:35
  • In addition, the code will still fail and throw a null exception if one or more cells in a row are `null` AND one or more cells in that same row are NOT `null.` In other words, the code will only work and not throw an exception… if ALL the cells in a row are `null` or ALL the cells in a row are NOT `null.` Any other case will throw a null exception. – JohnG Jan 20 '21 at 19:42
  • I'm absolutely not saying it's a good solution and think there is probably a far better solution out there! Exception may be being thrown (haven't actually tested this yet) but the highlight function is working to the end which it wasn't before. I'm open to better suggestions by all means (prob not hard!). – user2970749 Jan 21 '21 at 18:27
0

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.

JohnG
  • 9,259
  • 2
  • 20
  • 29
  • Firstly no offence taken at all! I knew there was a better solution somewhere to be found. Secondly, thank you for your in-depth explanation and examples. These work perfectly and I have replaced the code I had with this solution. I agree this is much better. Many thanks for your effort here. – user2970749 Jan 23 '21 at 13:12