System.NullReferenceException based on List

Question

I'm trying to loop through rows and get their Indexes (Primary Keys from SQL). I'm getting a NRE on "this.SelectRowIndexes.Add(ResourceKey)" I can't figure out why it would matter and how can I fix this?

CODE:

    private void GetIndexes()
    {
        List<int> SelectRowIndexes = new List<int>();
        for (int i = 0; i < gridViewRecords.Rows.Count; i++)
        {
            DataRowView drv = (DataRowView)gridViewRecords.CurrentRow.DataBoundItem;
            DataRow selectedRow = drv.Row;
            ResourceKey = Convert.ToInt32(selectedRow["ResourceAndInstallKey"]);
            this.SelectRowIndexes.Add(ResourceKey);

        }
    }

I also have it up in my class (this has been a ton of troubleshooting, so my code looks terrible)

    public List<int> SelectRowIndexes { get; set; }

I had this prior. Several of the answers quoted this code. I changed mine because the if-else was actually used for something else, which has now been deleted

if (this.SelectRowIndexes == null)
{
    this.SelectRowIndexes.Add(ResourceKey);
}
else
{
    this.SelectRowIndexes.Add(ResourceKey);
}

score 2 · Answer 1 · answered May 06 '15 at 16:21

2

If SelectRowIndexes is null then you can't add anything to the list. You first need to initialize an empty list with

this.SelectRowIndexes = new List<int>();

answered May 06 '15 at 16:21

Roemer

2,012
1
24
26

Now with the updated code its the line: List SelectRowIndexes = new List(); remove the List in front of it to initialize the property of the class instead of a local variable. – Roemer May 06 '15 at 16:27

Janne Matikainen · Answer 2 · 2015-05-06T16:28:34.427

You instantiate a local variable

List<int> SelectRowIndexes = new List<int>();

and then you are adding to your class property/field this.SelectedRowIndexes which you are most likely not assigning anywhere and it is null and you get NRE.

this.SelectRowIndexes.Add(ResourceKey);

Change this so

private void GetIndexes()
{
    this.SelectRowIndexes = new List<int>();
    for (int i = 0; i < gridViewRecords.Rows.Count; i++)
    {
        DataRowView drv = (DataRowView)gridViewRecords.CurrentRow.DataBoundItem;
        DataRow selectedRow = drv.Row;
        ResourceKey = Convert.ToInt32(selectedRow["ResourceAndInstallKey"]);
        this.SelectRowIndexes.Add(ResourceKey);
    }
}

Initially I didn't have the if statement. – Kevin Fischer May 06 '15 at 16:21 — Kevin Fischer, May 06 '15 at 16:21

Jon Skeet · Accepted Answer · 2015-05-06T16:46:46.097

1

What do you actually want to do if this.SelectRowIndexes is null? Currently you're just unconditionally calling Add on it, because both branches of your if statement do the same thing.

Note that it definitely wouldn't be null if you'd assigned a new value to it - but instead in this line, you're declaring a new local variable called SelectRowIndexes:

List<int> SelectRowIndexes = new List<int>();

... which you're then completely ignoring. Perhaps you meant to set the value of the property/field instead?

SelectRowIndexes = new List<int>();

With that change, you should avoid the exception - but you'll still have the basically-broken code. You should almost certainly just get rid of the if check... it's not doing you any good right now.

However, I would suggest that you probably should be declaring a separate local variable for resourceKey - the fact that you're updating an instance variable in a loop is somewhat peculiar... as is the fact that you're not using your loop index at all... you're doing the same thing for each iteration of the loop, using the current row rather than row i. Is that deliberate?

Fundamentally, you might well want to start this code again... it looks like you might just want to use LINQ:

private void GetIndexes()
{
    SelectRowIndexes = gridViewRecords.Rows
        .Cast<DataRowView>()
        .Select(drv => (int) drv.Row["ResourceAndInstallKey"])
        .ToList();
}

edited May 06 '15 at 16:46

answered May 06 '15 at 16:21

Jon Skeet

1,421,763
867
9,128
9,194

the if statement was something I was trying on a different piece, which is gone now. I'll update the code now – Kevin Fischer May 06 '15 at 16:25
I put List<> instead of this. Thank you. I should have known that. – Kevin Fischer May 06 '15 at 16:27
@KevinFischer: Well you don't need `this` at all unless you've got a local variable with the same name in scope... – Jon Skeet May 06 '15 at 16:34
Good point. My Senior Dev likes it with a "this". Their protocols. Thank you again – Kevin Fischer May 06 '15 at 16:40
@KevinFischer: It's a bad idea to edit the code in the question *after* a load of answers have been posted. You've now invalidated the points of various answers. I'll edit mine, but it would have been better if you'd left it as it was... – Jon Skeet May 06 '15 at 16:41
I added the old code to try to keep the previous answers associated with the post. – Kevin Fischer May 06 '15 at 16:45

score 1 · Answer 4 · answered May 06 '15 at 16:22

1

The this keyword allows you to access a member named SelectRowIndexes within the scope of the class. That member is probably not initialized. Remove the List<int> type declaration and it will be perfect.

private List<int> SelectRowIndexes;
private void GetIndexes()
{
    SelectRowIndexes = new List<int>();
    for (int i = 0; i < gridViewRecords.Rows.Count; i++)
    {
        DataRowView drv = (DataRowView)gridViewRecords.CurrentRow.DataBoundItem;
        DataRow selectedRow = drv.Row;
        ResourceKey = Convert.ToInt32(selectedRow["ResourceAndInstallKey"]);
        if (this.SelectRowIndexes == null)
        {
            this.SelectRowIndexes.Add(ResourceKey);
        }
        else
        {
            this.SelectRowIndexes.Add(ResourceKey);
        }
    }
}

answered May 06 '15 at 16:22

scottyeatscode

606
3
11

Your code is the same for both paths of the if() – cdkMoose May 06 '15 at 16:30
@cdkMoose that's correct. I just copypastad his code and patched it. – scottyeatscode May 06 '15 at 16:35
@cdkMoose it appears he has since edited his original post to remove the redundant if-else. I'll leave it in my answer as a form of eternal documentation. ;P – scottyeatscode May 06 '15 at 16:37
I see that Kevin edited the code in the posting after your answer. If you are going to post a code solution though, it should still be one that works :) The code in your answer will still throw an NRE. – cdkMoose May 06 '15 at 16:40
@cdkMoose, actually he did. If you check the edit for his post, you will see that I really copypastad his original code. – scottyeatscode May 06 '15 at 16:41
Actually, if you check my code again, you'll see that it doesn't throw any exceptions because it IS being initialized at the beginning of the method. – scottyeatscode May 06 '15 at 16:42

System.NullReferenceException based on List

4 Answers4