struberg commented on PR #1328:
URL: https://github.com/apache/commons-lang/pull/1328#issuecomment-2542345605

   > I agree. All the `reflectionX` methods have a warning:
   
   Well this is way more subtly weird right now.
   
   Imagine the following code:
   ```
           Map<String, String> a = new HashMap<>(20);
           a.put("A", "A");
   
           Map<String, String> b = new HashMap<>();
           b.put("A", "A");
   ```
   Here we have the case that `a.equals(b)` returns `true`. Because that's the 
contract of Maps!
   But when you use `new EqualsBuilder().setTestRecursive(true).append(a, 
b).isEquals()` then you'll get `false`.
   That's because by giving a different initial array size, we'll get a 
different *internal* (!) structure (bigger internal arrays, etc). 
   
   But that means that right now the EqualsBuilder (even in Java8) does not 
even adheres to the contract of the Map interface. Because those internal 
details should not matter. Even if I would not have set an initial size - what 
if I add 30 elements to one of the 2 maps, but then remove everything but the 
"A" entry again? It will be equals according to the Map contract, but not for 
the EqualsBuilder. Looks rather broken to me.
   
   One might argue that that's exactly how all the reflection stuff in 
commons-lang was designed from the beginning.
   Otoh there are already a few places where Maps, Collections, etc get a 
special treatment. So why not also here?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to