[02/03] Variables getting mixed up?

[Guiseppe]

This is driving my absolutely crazy:

Private cardPlayed(4) As Card
 
    ...
 
    Private Function getHighestCardOnTable() As String
        Dim backupCards(4) As Card
 
        For i As Int16 = 1 To 4
            If cardPlayed(i) Is Nothing Then
                backupCards(i) = New Card(0)
            Else
                backupCards(i) = cardPlayed(i)
 
                Dim sumToAdd As Int32 = 0
                If backupCards(i).SuitCard = troef.SuitCard Then sumToAdd += 5000
                If backupCards(i).SuitCard = suit Then sumToAdd += 1000
 
                backupCards(i).ValueCard += sumToAdd
            End If
        Next
 
        ... 
 
        return ...
    End Function

The function uses an array of 4 cards to calculate which one is the highest.
If a card belongs to trumps, its value is raised by 5000.
If a card belongs to the suit that came out first, its value is raised by 1000.

I'm using backupCards(4) so I still retain the old value (without the raises).
Or at least that's what I hoped...

On this line:

backupCards(i).ValueCard += sumToAdd

.. where I want the value of backupCards() to raise... it raises both the values of backupCards and cardPlayed()

[Half]

After 30 km bike ride a suggestion on the fly

Dim backupCards(4) As New Card

[jmcilhinney]

Can I start by pointing out that this:

Private cardPlayed(4) As Card

declares an array of FIVE cards, not FOUR. .NET arrays are zero-based, so that creates an array with elements at indices 0, 1, 2, 3 and 4, which is FIVE elements.

Your issue arises because, I assume, Card is a class, i.e. a reference type. That means that after this:

backupCards(i) = cardPlayed(i)

both arrays contain a reference to the same object. If you change one then you change the other because they are one and the same. If I give your mother a bunch of flowers, then I give your father's wife a bunch of flowers, how many bunches of flowers does she have? I gave flowers to two different references to the same person, so the one person got both bunches of flowers. That's how reference type objects work, and you really need to come to terms with that quickly.

[Shaggy Hiker]

Your issue arises because, I assume, Card is a class, i.e. a reference type. That means that after this:

backupCards(i) = cardPlayed(i)

both arrays contain a reference to the same object. If you change one then you change the other because they are one and the same. If I give your mother a bunch of flowers, then I give your father's wife a bunch of flowers, how many bunches of flowers does she have? I gave flowers to two different references to the same person, so the one person got both bunches of flowers. That's how reference type objects work, and you really need to come to terms with that quickly.

How long do you sit around trying to find new metaphors?

Isn't that a nice feature? In C++, this is fixed by using a copy constructor. No such creature exists in VB.NET. However, you need to do the same thing. The problem is that you are doing what is called a 'shallow' copy, just copying the address of the object, not the object itself. You need a deep copy. Oddly, VB.NET makes a shallow copy the default for classes, and deep copies the default for structures, but gives no simple means for doing a deep copy of a class (and why bother with a shallow copy of a structure?).

Look into the ICloneable interface, but don't expect that it does the job for you. I implemented a Copy() function for some classes, but Clone seems intended to allow the same thing. Also look up Deep copy and Shallow Copy in MSDN for more information.

[jmcilhinney]

It makes complete sense that shallow copy is the default. If the default was a deep copy then copying an object would, by default, create a copy of every object it referenced and every object they referenced and every object they referenced and so on. A recursive deep copy could make copies of hundreds or even thousands of objects.

[ZaNi]

Can I start by pointing out that this:

Private cardPlayed(4) As Card

declares an array of FIVE cards, not FOUR. .NET arrays are zero-based, so that creates an array with elements at indices 0, 1, 2, 3 and 4, which is FIVE elements.

... I was going to disagree and say that cardPlayed(4) has indexes from 0-3, but as it turns out (through testing), you are right (I shouldn't have doubted the word of the great jmcilhinney). This takes me by surprise, because in all the programming I've done in the past (mostly c++), arrays are declared with their size, not their final index... It seems silly to me to do it differently now... How odd...

[Shaggy Hiker]

I'm not objecting to objects having a default shallow copy, but there is no obvious copy constructor. I'm surprised at this because there is such an obvious need for one. A default copy constructor would be possible, though it could also be dangerous for the same reasons you mention.

[Guiseppe]

I'm perfectly aware that it creates an array of 5 cards instead of 4. I don't use the 0 index in this case.

Not really the best way to program, I know, but in this case I find it much easier to debug since the numbers 1 to 4 correnspond to player 1 to 4.
I might change it after everything is completed, if I can be bothered.

About the copy... seems strange I enver encountered it before. I figured it was something about being reference to the same object but I couldn't see why.
So, what are my options now?
Write a copy function in the Card class that creates a new Card() with all the appropriate values and returns the new Card?

[jmcilhinney]

As Shaggy already suggested, implement the IClonable interface and provide a Clone method.

As for the arrays, in my opinion what you're doing is bad programming.

if I can be bothered

What the...? Do the right thing in the first palce and you will get in good habits and then doing the right thing becomes second nature. If I had players 1 to 4 I would use myArray(playerNumber - 1) without even thinking about it. When you do dodgy things like that you can end up getting caught out, like a loop that visits every element and crashing because one of the elements is a null reference. What you do is up to you, of course, but if I was your teacher or your employer I'd be getting you out of that habit quick smart.

[Guiseppe]

It's not like it I access the array once or twice, but dozens of times.
And there are 5 or so arrays just like that.

I started out using 0 to 3, and noticed I forgot to add 1 sometimes, sometimes resulting in not a crash but in weird situations that made me spend too much time on debugging.

Why I might not be bothered? Changing it back would take some time, and I don't think having those unused spaces takes up too much memory.
But then again, this is neither work for my employer or school, I'd feel obliged to do it correct there.

[jmcilhinney]

Like I said, do the right thing all the time and it becomes second nature. Do it different ways at different times and you increase the chance that you'll confuse yourself. It must be exhausting to type "+ 1" or "- 1" so many times.

[Shaggy Hiker]

It is a good habit to get into, but it is also a VERY common error, so I agree with both of you on this. Base 0 indexing is something that is notorious for causing little bugs. However, so is index arithmetic, so you can have problems even with Base 1.

By the way, IClonable creates a stub of a function, and I can't see any really obvious reason to use it in this case (as opposed to naming it Copy(), or some such). However, by implementing the interface, you are announcing to the compiler that there exists a function Clone() that returns a certain object, and the compiler can work with that. I tried to find an example of using Clone(), but all of the times I have used it, it was so specialized that it made a very poor example of the general case.