Tbh, I was really surprised to find out that in Java8 the code did indeed also introspect internal fields of all those java.util.* foundation classes. It's imo pure luck that it did work so well for a decade for anything Set, List, Map, etc.
Working on a patch with all those discussed changes. LieGrue, strub > Am 08.03.2024 um 13:19 schrieb Gary Gregory <garydgreg...@gmail.com>: > > The next question is whether any of this should be mentioned/recorded in > the Javadoc or at least in a code comment. > > Gary > > On Fri, Mar 8, 2024, 5:24 AM Mark Struberg <strub...@yahoo.de.invalid> > wrote: > >> Hi Gary! >> >> Yes, this would be really slow. Plus after further thinking about it, I >> guess it doesn't add anything over the required existing behaviour imo. >> Until now reflectionEquals did simply dig into the Class.getDeclaredFields >> of those java.util.* classes. So it only did compare the EXAKT same >> situations. having differences in internal hash codes or whatever would >> result in detecting a difference. So probably just iterating over the >> entries is perfectly fine for now. >> Same for Maps. Not sure though, whether we should just iterate over the >> entries and compare those, or do a map lookup (which would require equals >> again). >> >> Btw, I thought again about the 'customEquals' part. Maybe we cannot rely >> on it fully, means if it returns false it doesn't mean they are really >> different. Otoh IF it returns true, then we can take it for granted, that >> those two are the same. And further thinking about it: what if we *always* >> invoke equals() first, and if it returns true -> return true and skip the >> rest for this tree? >> >> LieGrue, >> strub >> >> >>> Am 07.03.2024 um 14:55 schrieb Gary D. Gregory <ggreg...@apache.org>: >>> >>> On 2024/03/07 06:58:30 Mark Struberg wrote: >>>> The question to me is how we can make it more robust. >>>> In a Collection (but actually also in most lists) the order in which >> you get the values (Iterator or get(i)) is not deterministic. It can be >> different in one list than in another - even if they contain the exact same >> items. >>> >>> Hm, so to iterate through Lists in parallel would work but not with Sets. >>> >>>> >>>> Not yet sure how to work around this. We can probably try to sort it >> first, but then again, if they do not implement Comparable it won't help >> much. Or do a containsElement based on reflection as well - but that would >> be rather slow. >>> >>> This is one of those: If you want support for the feature, it'll work, >> but it'll be slow because there is no other way to do it (for now if ever). >>> >>> Gary >>> >>>> >>>> Same with Maps. There is a good reason why the key at least should >> implement equals/hashCode. If this is not the case, then we are not able to >> implement this properly I fear. >>>> >>>> Any ideas? >>>> >>>> LieGrue, >>>> strub >>>> >>>>> Am 06.03.2024 um 15:53 schrieb Gary Gregory <garydgreg...@gmail.com>: >>>>> >>>>> Ah, right, custom "non-equalable" _inside_ Collections and Maps... >>>>> >>>>> For the diff, I'd suggest you test and iterable over a Collection >>>>> instead of a List. >>>>> >>>>> Then you'd need a separate test and traversal for Map instances. >>>>> >>>>> (Still no common super-interface in Java 21 for Collections and >> Maps...) >>>>> >>>>> Gary >>>>> >>>>> On Wed, Mar 6, 2024 at 7:40 AM Mark Struberg <strub...@yahoo.de.invalid> >> wrote: >>>>>> >>>>>> Hi Gregory! >>>>>> >>>>>> I did try this out and figured that I didn't think it though. Maybe I >> need to go a few steps back and explain the problem: >>>>>> >>>>>> I have the following constellation >>>>>> >>>>>> public class SomeInnerDTO {int field..} // NOT implements equals! >>>>>> public class TheOuterDTO{ List<SomeInnerDTO> innerList;..} >>>>>> >>>>>> My problem is that reflectionEquals (which I use in a unit test) >> tries to introspect fields even in java.util.classes. And for getting the >> values of those classes it tries to call a setAccessible(true); >>>>>> And obviously here it fails. There is a ticket already open [1] which >> sugggests to use trySetAccessible. But I fear that will still do nothing >> and you won't get access to those inner knowledge inside >> java.util.LinkedList etc. >>>>>> >>>>>> And using equals() on the List sadly won't help either, as the >> SomeInnerDTO would also get compared with equals(), but that will obviously >> use identity comparison only :( >>>>>> >>>>>> >>>>>> What I did for now (running all tests with a few projects right now, >> but looks promising): >>>>>> >>>>>> diff --git >> a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java >> b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java >>>>>> index ff5276b04..cf878da40 100644 >>>>>> --- >> a/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java >>>>>> +++ >> b/src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java >>>>>> @@ -978,6 +978,16 @@ public EqualsBuilder reflectionAppend(final >> Object lhs, final Object rhs) { >>>>>> if (bypassReflectionClasses != null >>>>>> && (bypassReflectionClasses.contains(lhsClass) || >> bypassReflectionClasses.contains(rhsClass))) { >>>>>> isEquals = lhs.equals(rhs); >>>>>> + } else if (testClass.isAssignableFrom(List.class)) { >>>>>> + List lList = (List) lhs; >>>>>> + List rList = (List) rhs; >>>>>> + if (lList.size() != rList.size()) { >>>>>> + isEquals = false; >>>>>> + return this; >>>>>> + } >>>>>> + for (int i = 0; i < lList.size(); i++) { >>>>>> + reflectionAppend(lList.get(i), rList.get(i)); >>>>>> + } >>>>>> } else { >>>>>> >>>>>> I'm rather sure this is still not enough and there are plenty other >> cases to consider. Like e.g. handling Maps etc. >>>>>> But at least that's the direction I try to approach it right now. And >> of course this new part should potentially also be enabled by a flag... >>>>>> >>>>>> Will keep you updated. >>>>>> >>>>>> txs and LieGrue, >>>>>> strub >>>>>> >>>>>> >>>>>> [1] https://issues.apache.org/jira/browse/LANG-1711 >>>>>> >>>>>>> Am 06.03.2024 um 13:18 schrieb Gary Gregory <garydgreg...@gmail.com >>> : >>>>>>> >>>>>>> This sounds like a good idea to try. I would call the option >> something else >>>>>>> though. We would not skip calling equals if it is defined right? How >> about >>>>>>> "useEqualsIfPresent". >>>>>>> >>>>>>> Gary >>>>>>> >>>>>>> On Wed, Mar 6, 2024, 5:03 AM Mark Struberg <strub...@yahoo.de.invalid >>> >>>>>>> wrote: >>>>>>> >>>>>>>> Hi! >>>>>>>> >>>>>>>> I have a question about EqualsBuilder#reflectionEquals. From Java9 >> onwards >>>>>>>> we get more and more nasty module problems. Mainly because the code >> tries >>>>>>>> to recurse into java.util.* classes as well. >>>>>>>> I know that I can use setBypassReflectionClasses for those. But >> wouldn't >>>>>>>> it be fine to have an additional switch to 'skipOnCustomEquals' or >> similar? >>>>>>>> >>>>>>>> The idea is to only use reflection on classes which do not provide >> their >>>>>>>> own equals method. One can test this imo rather easily by checking >> whether >>>>>>>> classInQuestion.getMethod("equals", >> Object.class).getDeclaringClass() != >>>>>>>> Object.class >>>>>>>> Do that for lhs and rhs and if both fit the criteria -> invoke >> equals >>>>>>>> >>>>>>>> Wdyt of that idea? Worth trying or is there already a better >> solution? >>>>>>>> With the new flag we can make sure that we do not change the current >>>>>>>> behaviour for existing use cases. >>>>>>>> >>>>>>>> LieGrue, >>>>>>>> strub >>>>>>>> >>>>>>>> >>>>>>>> >> --------------------------------------------------------------------- >>>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>>>> >>>>>>>> >>>>>> >>>>>> >>>>>> --------------------------------------------------------------------- >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>>> >>>>> >>>>> --------------------------------------------------------------------- >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>>>> For additional commands, e-mail: dev-h...@commons.apache.org >>>>> >>>> >>>> >>>> --------------------------------------------------------------------- >>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org <mailto: >> dev-unsubscr...@commons.apache.org> >>>> For additional commands, e-mail: dev-h...@commons.apache.org <mailto: >> dev-h...@commons.apache.org> >>>> >>>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org <mailto: >> dev-unsubscr...@commons.apache.org> >>> For additional commands, e-mail: dev-h...@commons.apache.org <mailto: >> dev-h...@commons.apache.org> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org