Its equals implementation simply compares the content, following the contract for list equality. From the List.equals() javadoc:
...two lists are defined to be equal if they contain the same elements in the same order. This definition ensures that the equals method works properly across different implementations of the List interface.
However, there's a problem that NBT is faced with, having four list-like types. Comparing NbtList == NbtIntArray, you would always expect that to be false, as they represent two different serialized forms. But, according to the List contract, an empty NbtList should equal an empty NbtIntArray, since they have the same content.
Reflexive: for any non-null value x, x.equals(x) should return true.
Symmetric: for any non-null values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
Transitive: for any non-null values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
Consistent: for any non-null values x and y, multiple invocations of x.equals(y) consistently return true or consistently return false, provided no information used in equals comparisons on the objects is modified.
Never equal to null: for any non-null value x, x.equals(null) should return false.
Example:
nbtList == list, and
list == NbtIntArray, so ideally
nbtList == nbtIntArray, but it doesn't.
Potential solutions
Have the list-like NBT types implement Collection instead (or nothing at all, similar to Kotlin's Array classes)
Pros:
Unbeholden to the List equality contract
Allows for more NBT-specific collection semantics (e.g. how empty TAG_Lists can have a different element type encoded)
Cons: (inconvenience, which can be mostly alleviated by NbtList/Nbt*Array.asList() extensions)
Can't directly use as Lists
Lose access to some of Kotlin's list-specific stdlib extensions
May have to add member functions to mirror the methods in List
Have to consider making NbtCompound not implement Map for similar reasons
Add an equals(NbtTag?) overload to NbtTag
Pros:
Can retain the convenience of the types implementing List
Cons:
May not be possible to work with kotlin's Any?.equals() stdlib extension (since a similar NbtTag?.equals() would need to be explicitly imported, and that seems like a potentially major source of issue)
Overloading equals in general feels like a design smell, since values are either equal or they aren't, and it doesn't seem like something the caller should be concerned with
Leave the implementation how it is, accepting the broken transitivity requirement
Pros:
Keeps the same convenience
Cons:
Breaks assumptions made by List consumers, albeit in potentially uncommon cases
Possible conclusion
After having thought on this for admittedly way too much time, I'm leaning towards the first option and implementing nothing for the list-like types (and NbtCompound as well for consistency), leaving richer functionality to the kotlin stdlib List/Maps, and bridging the gap in convenience with asList() and asMap() extensions. Plus adding basic members like size and get() that are often used at the deserialization boundary.
History
Currently (v0.11), the
NbtList/Arrayclasses all implement theListinterface, following the example thatkotlinx-serialization-jsonset withJsonArray(implementingListfor convenience). https://github.com/Kotlin/kotlinx.serialization/blob/8a2c1c0e05ac9c77746141837f6d53d923e24d8a/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt#L191-L195Its
equalsimplementation simply compares the content, following the contract for list equality. From the List.equals() javadoc:However, there's a problem that NBT is faced with, having four list-like types. Comparing
NbtList == NbtIntArray, you would always expect that to befalse, as they represent two different serialized forms. But, according to theListcontract, an emptyNbtListshould equal an emptyNbtIntArray, since they have the same content.In v0.11, this is worked around by special-casing NbtTag comparisons, and only checking for the same
NbtTagtype if both areNbtTags. E.g.NbtList.equals(): https://github.com/BenWoodworth/knbt/blob/dba146f46b46dcbc2849c02ad21912e654deb869/src/commonMain/kotlin/NbtTag.kt#L125-L129So, for empty
list/nbtList/nbtIntArrays:Listcontract is satisfied for non-NBT types:list == nbtListandlist == nbtIntArraynbtList != nbtIntArrayThe remaining problem
This approach satisfies all of the requirements in the Any.equals() contract, except transitive:
Example:
nbtList == list, andlist == NbtIntArray, so ideallynbtList == nbtIntArray, but it doesn't.Potential solutions
Collectioninstead (or nothing at all, similar to Kotlin's Array classes)Listequality contractTAG_Lists can have a different element type encoded)NbtList/Nbt*Array.asList()extensions)ListsListNbtCompoundnot implementMapfor similar reasonsequals(NbtTag?)overload toNbtTagListAny?.equals()stdlib extension (since a similarNbtTag?.equals()would need to be explicitly imported, and that seems like a potentially major source of issue)equalsin general feels like a design smell, since values are either equal or they aren't, and it doesn't seem like something the caller should be concerned withListconsumers, albeit in potentially uncommon casesPossible conclusion
After having thought on this for admittedly way too much time, I'm leaning towards the first option and implementing nothing for the list-like types (and
NbtCompoundas well for consistency), leaving richer functionality to the kotlin stdlib List/Maps, and bridging the gap in convenience withasList()andasMap()extensions. Plus adding basic members likesizeandget()that are often used at the deserialization boundary.