Adding member to list overides previous members

Question

I need to read from an Excel file a list of dogs with their birth date, and store them in a list. The read works well, and every time I add a new dog to the list, the list count grows. So far so good.

The problem is that when I add the 2nd record, the first record is replaced by the first record content, so I have two identical records. And so on with every new record. In the end, I have many same records - all with the value of the last dog.

The dog class is:

Public Class DogClass
    Public Name As String
    Public Dob As Date
    Public Age As Integer
    Public Sex As String

    Public Sub setDogName(ByVal _name As String)
        Name = _name
    End Sub
    Public Sub setDogDOB(ByVal _dob As Date)
        Dob = _dob
    End Sub
    Public Sub setDogAge(ByVal _age As String)
        Age = _age
    End Sub
    Public Sub setDogSex(ByVal _sex As String)
        Sex = _sex
    End Sub

End Class

The class of the list is:

Public Class DogsListClass
    Public dogsList As New List(Of DogClass)

    Public Function DodgsCnt() As Integer
        DodgsCnt = dogsList.Count()
    End Function

    Public Function DogExsists(_dogName As String) As Boolean
        Dim res As Boolean = False
        For Each item As DogClass In dogsList
            If item.Name = _dogName Then
                res = True
            End If
        Next
        Return res
    End Function
    Public Sub AddDog(_dog As DogClass)
        dogsList.Add(_dog)
    End Sub

End Class

The calling:

Dim tdog As New DogClass
Dim DogsList As New DogsListClass
Do
    tdog.setDogName(MyExcel.Cells(row_cnt, col_cnt).text)
    tdog.setDogDOB(MyExcel.Cells(row_cnt, col_cnt + 1).value)
    DogsList.AddDog(tdog)
Loop

Any idea why the records are being overridden?

You have to create new instance of DogClass in every iteration, otherwise it keeps adding same object to list. And please, do not use Dim ... As New ... constructs - that way it is hard to follow object creation and deletion logic. — Arvo, Mar 06 '19 at 09:57
There is another problem: the code shows `Public Age As Integer` but a few lines later has `Public Sub setDogAge(ByVal _age As String)`. You can get Visual Studio to point out problems like that by using [`Option Strict On`](https://stackoverflow.com/a/29985039/1115360). Also, [Auto-Implemented Properties](https://learn.microsoft.com/en-us/dotnet/visual-basic/programming-guide/language-features/procedures/auto-implemented-properties) reduces the typing needed. — Andrew Morton, Mar 06 '19 at 10:41
Thanks Arvo. Solved the problem. I create an instance of DogClass every iteration and now the list is as it should be. I was sure that once the instance is added, a copy is added. But It seems the actual instance is linked to the list. Thank you! — YigalB, Mar 06 '19 at 12:28
Good point Andrew. I didn't catch yet because the code is not calling it yet. Now it is fixed. I like your idea about strict rules and added the first suggestion. However, I couldn't locate the 2nd one - will work on it later. — YigalB, Mar 06 '19 at 12:37
@Arvo - although the solution you suggested works perfectly, I still wonder why is it needed. If the procedure gets the parameter byVal, it is supposed to create a copy of it, and from that moment on the original variable is don't care. But it seems I am wrong - I just don;t understand why. — YigalB, Mar 07 '19 at 08:28
ByVal doesn't (and can't) create copy of object, it creates copy of object reference (address). — Arvo, Mar 07 '19 at 09:42
@Arvo: I was sure that this is ByRef (sending address, which allows to alter the original value in the source), unlike ByVal which delivers a copy. Anyhow, since it is as you said, I am wondering how can I add instances of tdog to a global list of tdogs? It seems that once the sub that includes the instances of tdogs is termonated, all values added on tdogs are gone from the global list. Does it require some kind of a static deceleration? — YigalB, Mar 09 '19 at 10:19
No. As far there exist any references to objects, they are not destroyed. Global list refers to dogs, thereby they don't get lost. — Arvo, Mar 11 '19 at 07:21

score 0 · Answer 1 · answered Mar 06 '19 at 07:22

0

DogsList variable must be declared outside the Do...Loop, otherwise you are creating a "New" DogsList at each iteration and, based on this code, you should end up having one single item in the collection, not many. Also, declaring DogsList in the loop, prevents you from using it outside in the rest of your code.

answered Mar 06 '19 at 07:22

A.Buonanni

101
5

Correct. I agree. the deceleration is before the loop. The mistake was mine while I was preparing the message. The variables are declared only once. – YigalB Mar 06 '19 at 08:15

Adding member to list overides previous members

1 Answers1