Object reference issue with copy of Data Class instance

I’ve found an interesting behaviour of .copy() method of data class instances. It seems to me there is a bug but I’m new in Kotlin and probably I’m just missing something.

Let’s take an example with two classes: player (holds a collection of cards) and card, which can be discarded from player’s collection. Each card has a reference to the current owner.

data class TestCard(var test : Int = 0) {

    var player : TestPlayer? = null

    fun discard() {
        if (player != null) {
            if (player?.cards!!.contains(this)) {
                player?.cards!!.remove(this)
            }
        }

        this.player = null
    }
}

class TestPlayer {

    val cards : MutableList<TestCard> = mutableListOf()

}

and then let’s give some card copies to a player and later discard one of them.

    @Test
    fun testDataClassFail() {
        val player = TestPlayer()
        val cardBasic = TestCard()

        player.cards.add(cardBasic.copy())
        player.cards.add(cardBasic.copy())

        player.cards.forEach {
            it.player = player
        }

        //check that objects are different
        assertFalse{ player.cards[0] === player.cards[1] } // pass

        player.cards[1].discard()

        assertEquals(1, player.cards.size) // pass
        assertNotNull(player.cards[0].player) // fail!
    }

For some reason, when modifying the second card in the list, we automatically modify the first one. It’s a mystery to me why it happens. I’m using this inside .discard(), which must guarantee modification of only one object.

I suppose that the issue is somewhere under the hood of data class instances equality. If we make the copies unequal in terms of .equal() in-built method, the issue disappears:

    @Test
    fun testDataClassPass() {
        val player = TestPlayer()
        val cardBasic = TestCard()

        player.cards.add(cardBasic.copy())
        player.cards.add(cardBasic.copy())

        player.cards.forEach {
            it.player = player
        }

        // make data class objects unequal
        player.cards[0].test = 0
        player.cards[1].test = 1

        //check that objects are different
        assertFalse{ player.cards[0] === player.cards[1] } // pass

        player.cards[1].discard()

        assertEquals(1, player.cards.size) // pass
        assertNotNull(player.cards[0].player) // pass!
    }

The issue you describe is in fact unrelated to the copy method, but rather to the behavior of List.remove().

As you already noticed, data classes have the equals method auto-generated, based on properties declared in primary constructor. Thus, in your first example both cards are considered “equal”.

On JVM Kotlin’s (Mutable)List is actually backed by java.util.List, so let’s have a look into its documentation.

Removes the first occurrence of the specified element from this list, if it is present (optional operation). If this list does not contain the element, it is unchanged. More formally, removes the element with the lowest index i such that (o==null ? get(i)==null : o.equals(get(i))) (if such an element exists). Returns true if this list contained the specified element (or equivalently, if this list changed as a result of the call).

As you can see, the removal is based on equality as defined by equals, not on reference equality. So, even through you are calling remove(this) it is not guaranteed this very instance will be removed, but rather the first “equal” one, as it happens in your example.

2 Likes

@akurczak, thank you for the doc reference and explanation! Indeed, the root cause is related to equals() but I was looking into the wrong direction. Important lesson about List behaviour.