> Am 14.12.2024 um 12:00 schrieb Piotr P. Karwasz <pi...@mailing.copernik.eu>: > > Hi Mark, > > On 9.12.2024 17:37, Mark Struberg wrote: >> c) try using this on a class which contains e.g. an ArrayList, a Set or a >> Map. It will e.g. use List#equals() which also does equals() on it's items >> (btw, I explained this to you already...). Which is NOT what users expect. >> Instead it should iterate over the List and use ReflectionEquals on the >> elements. This is exactly what the PR does (amongst other things). > > Why would a user expect to use `reflectionEquals` on the elements of an > internal list field?
Because the javaDoc says exactly that: https://github.com/apache/commons-lang/blob/95d3485cd4312d8e1a846887b0b555eec4f7417b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java#L199 > IMHO, if you run `reflectionEquals` on two objects of type A: > > * the caller must ensure that the fields of A are accessible. This > requirement didn't change: previously the caller had to set the appropriate > `SecurityManager` permissions, now he needs to set the appropriate JPMS > permissions. The JavaDoc still says nothing about the Java Module System. > * internal fields that are NOT of type A or Collection<A>, should be compared > using `equals()`. Oh no, it even must do that according to the JavaDocs IF the testRecursive parameter is set. Which my PR does. Maybe I have a glitch also affecting non-recursive behaviour? Then we certainly need to fix this in my PR. But this doesn't change the requirement for the setRecursive case. > > * internal fields that are of type A or Collection<A>, can be compared using > `reflectionEquals` if the users requests it. The point is that the JavaDoc very prominently says that the Java equals() and hashCode() contracts will be strongly met. https://github.com/apache/commons-lang/blob/95d3485cd4312d8e1a846887b0b555eec4f7417b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java#L35 But with every Collection and Map we break the equals() contract. Imo that's not a good thing. Others may disagree, and I will certainly follow the vote of the majority. But then we must also explicitly point this out in the JavaDocs and scrap the whole reference to Effective Java and the equals/hashCode contracts. LieGrue, strub
