util: handle duplicate elements in Collection.containsExactlyInAnyOrder()#6318
Conversation
…er() containsAll() only checked for unique element presence, not frequencies. Changed to use count maps for duplicate element comparison.
|
Right, thank you! That reminds me, I wanted to get rid of this function. IIRC it is only used in tests, for things like assertTrue(
actualResult.containsExactlyInAnyOrder(expectedResult)
)while I'd rather use this assertEquals(
expectedResult.toSet(),
actualResult.toSet()
)I use this at a few places already. Why is it better? Because if it fails, I get a better feedback error message from the unit test. And yeah, I realize now that it has the same issue as the current implementation of that function... hmm... 🤔 So maybe we just need a new function ...which uses your implementation but will print a useful error when the test fails. |
Yeah I was curious if that's even an issue, so I searched for uses before I changed it. But there were uses in the codebase as well in That's also the reason I've chosen this more per formant variant which should run at O(n) space, O(n) time so it works for large data lists as well. |
|
Added some tests |
|
I'll add the function which is described in the TODO/in your comment, I just can't complete it today anymore. |
westnordost
left a comment
westnordost
left a comment
There was a problem hiding this comment.
Well, I guess it could be merged in the current state, too
2 participants
containsAll() only checked for unique element presence, not frequencies. Changed to use count maps for duplicate element comparison.
The original implementation would incorrectly return true for collections
like [1, 2, 2, 3] and [1, 2, 3, 3] because:
The new implementation uses groupingBy().eachCount() to create frequency maps ({1=1, 2=2, 3=1} vs {1=1, 2=1, 3=2}), which correctly identifies these collections as different due to varying element frequencies.
This ensures the function behaves as its name suggests - checking for exact content equality regardless of order.