Object reference not set to instance and I got mad

[jamshed ahmed]

I just want to delete if column qty contains value 0 then delete that row
but i get error after delte six records it says object refrence not set to instance
there are total 12 records it delts six record without error

 For i As Integer = 0 To dt.Rows.Count - 1

    value = DgvStock.Item("Qty", i).Value.ToString
    If value = 0 Then
        DgvStock.Rows.RemoveAt(i)
        DgvStock.Refresh()
    End If
    DgvStock.DataSource = dt

  next

[djjeavons]

Hi

Are you sure it is stating Object Reference Not Set as I would expect it to throw an System.ArgumentOutOfRangeException.

The reason being is that you are loop through each row of the DataTable via an Integer index (so always 1 - 12 in your example), but each time you come across a 0 quantity you remove a row from the DataGridView which means that it's row count goes down. So when you only have six rows left but are on index 7 of your for loop, it's gonna crash.

This is why you should never modify a collection within a loop, but instead look at alternatives such as copying the data to a temporary source.

Anothing thing I noticed is that you are treating the value variable as a string during assignment value = DgvStock.Item("Qty", i).Value.ToString but then treating it as an Integer during test If value = 0 Then. This is not good practice and can lead to unexpected results during runtime (some of the harder bugs to track down). I would recommend that you use Option Strict On so that these types of issues are caught at compile time.

Finally, a solution to your problem could be to use the CopyToDataTable method which is available from .NET 3.5 upwards which allows you to query your table and assign the results back to another DataTable. For example:

        Dim stock = From dt In dt.AsEnumerable() Where dt.Field(Of Integer)("Qty") > 0 Select dt
        Dim newTable As DataTable = stock.CopyToDataTable

        DgvStock.DataSource = newTable

HTH

[Minimalist]

Yes you need to assign your value properly. Otherwise start your loop at the top and go down. That should work.
For i As Integer = dt.Rows.Count - 1 To 0 step -1

[samir_ibrahim]

you can try the below

DgvStock.DataSource = dt

dt.AsEnumerable().Where(Function(r) r.Field(Of Integer)("qty") =0).ToList().ForEach(sub(x) x.Delete())

[Reverend Jim]

That line of code may be correct (I'm not going to parse it) but it is so unclear I would be appalled if I saw it in production code. In my experience, clever code is almost always unmaintainable.

[djjeavons]

Can I ask who you are referring too? I see one up vote so I assume mine but could be wrong.

Sorry I also see one down vote.

[Reverend Jim]

Sorry. I should have quoted. I was referring to

dt.AsEnumerable().Where(Function(r) r.Field(Of Integer)("qty") =0).ToList().ForEach(sub(x) x.Delete())

[samir_ibrahim]

@Reverend

Sorry if you consider this code complex and hard to maintain but is very easy, and downvote? I guess i need a lawyer lol

What you see I consider it as one of Lambada power to deal with rows in datatable in very effecient way that is the same as :

Dim RowsToDelete =  dt.select("Qty = 0")
For each RowToDelete In RowsToDelete
    RowToDelete.Delete
Next

Althought I like @Minimalist idea (+1) of making loop reverse, but sometimes you need to select records that cannot be done by doing loop, Lambada is your choice.

Any way, where is my lawyer?

[Reverend Jim]

The difference is that it is immediately obvious what the longer version does at a glance. In a recent survey, professional developers were asked "what makes good code?" The top nine responses were

78% - Readable
29% - Testable/Tested
24% - Simple
24% - Works
21% - Maintainable
18% - Documented
18% - Extendable/Reusable
17% - Self documented
15% - Clean

Readability was by far the most important trait. Concise was way down the list at 6%. Clever didn't even make the list. As someone who spent most of his career maintaining/modifying code written by other people I have to agree with the results of the survey.

[samir_ibrahim]

@Jim
I don't agree on the above statistics and I don't agree on your comment on my code. Why?

First, It seems the one who made this statistics like the slow motion activity because he didnot mentioned "Speed and performance". Where I will use code written in machine language (01) in my project if it proves its faster by > 20% from a code that easy to maintain.

Second, my code is simple to people who use alot of database manipulation

Third, I made a test on a SQL Server table I had, I get 10,000 rows where I have 2477 rows has p_fcat = 10 and tried these both codes

Me.dataGridView1.DataSource = DT1
Dim sw As New Stopwatch
sw.Start
DT1.AsEnumerable.Where(Function(f) f("p_fcat") = 10).ToList.ForEach(Sub(s) s.Delete)
sw.Stop
Dim ts As TimeSpan = sw.Elapsed
Button4.Text = ts.Minutes.ToString + ":" + ts.Seconds.ToString + ":" + ts.Milliseconds.ToString

result = 0:0:480

dataGridView1.DataSource = dt2
Dim sw As New Stopwatch
sw.Start
For i As Integer = dt2.Rows.Count - 1 to 0 Step -1 
    Dim value = dt2(i)("p_fcat")
    If value = 10 Then
        dt2.Rows.Remove(dt2.rows(i))
    End If
Next
sw.Stop
 Dim ts As TimeSpan = sw.Elapsed
Button5.Text = ts.Minutes.ToString + ":" + ts.Seconds.ToString + ":" + ts.Milliseconds.ToString

result : 0:2:459

Lambada is faster than loop by 5 times

I will chose Lambada in my projects if I don't know anything about it even if I don't understand how it work (but I do :) )

[Reverend Jim]

I suggest a compromise for anyone wanting to use a lambda that is not intuitively obvious. Use the lambda but add a comment explaining what the statement does. Have pity on the poor maintenance programmer who inherits your code (especially one who may not be as clever as you).

An even better compromise would be to include both as

'This code deletes a set of rows but is less efficient than
'the following lambda version

'For each Row In dt.select("Qty = 0")
'    Row.Delete
'Next

'The following lambda version does exactly the same thing but
'takes a fraction of the time

dt.AsEnumerable().Where(Function(r) r.Field(Of Integer)("qty")=0).ToList().ForEach(sub(x) x.Delete())

This preserves clarity and efficiency and provides a little knowledge for the next programmer.