Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-08 Thread Mark Struberg
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 :
> 
> 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 
> 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 :
>>> 
>>> 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 :
> 
> 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 
>> 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 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
>>  

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-08 Thread Gary Gregory
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 
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 :
> >
> > 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 :
> >>>
> >>> 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 
> 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 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;
>  +   

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-08 Thread Mark Struberg
Hi Daniel!

Yes, you are right. As written in my mail to Gary: I think it would be enough 
for now if we'd come to the same results as before the module privacy 
restrictions. If we do better later on it is an optional bonus.

LieGrue,
strub


> Am 07.03.2024 um 15:39 schrieb Daniel Watson :
> 
> One comment about the collection comparison...
> 
> For any collection that actually implements List, equality should *include*
> order, not attempt to ignore it, right? The contract of a list is that the
> order is consistent, and two lists with the same items in different order,
> should not be considered equal.
> 
> e.g for List:
> 
> Best case scenario - The lengths are different and you dont have to check
> any equalities
> Worst case scenario - every item is equal and you have to iterate both
> lists completely
> Remaining Cases  - Item [i] is different and you dont have to check
> anything past that
> 
> (For pretty much any collection I think the best case is the same)
> 
> For sets, maybe it could be optimized by creating a new collection and
> removing items as you find them, so each step necessarily gets smaller? Not
> sure about this though.
> 
> 
> 
> On Thu, Mar 7, 2024 at 8:55 AM Gary D. Gregory  wrote:
> 
>> 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 :
 
 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 
>> 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 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 < 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-08 Thread Mark Struberg
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 :
> 
> 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 :
>>> 
>>> 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  
>>> 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 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 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-07 Thread Daniel Watson
One comment about the collection comparison...

For any collection that actually implements List, equality should *include*
order, not attempt to ignore it, right? The contract of a list is that the
order is consistent, and two lists with the same items in different order,
should not be considered equal.

e.g for List:

Best case scenario - The lengths are different and you dont have to check
any equalities
Worst case scenario - every item is equal and you have to iterate both
lists completely
Remaining Cases  - Item [i] is different and you dont have to check
anything past that

(For pretty much any collection I think the best case is the same)

For sets, maybe it could be optimized by creating a new collection and
removing items as you find them, so each step necessarily gets smaller? Not
sure about this though.



On Thu, Mar 7, 2024 at 8:55 AM Gary D. Gregory  wrote:

> 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 :
> > >
> > > 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 
> 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 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 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-07 Thread Gary D. Gregory
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 :
> > 
> > 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  
> > 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 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 :
> >>> 
> >>> 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 
> >>> 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 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-07 Thread sebb
Could one calculate the hashes for each entry, sort them, and then
generate the hash for the collection?

On Thu, 7 Mar 2024 at 06:59, 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.
>
> 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.
>
> 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 :
> >
> > 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  
> > 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 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 :
> >>>
> >>> 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 
> >>> 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 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-06 Thread Mark Struberg
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.

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.

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 :
> 
> 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  
> 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 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 :
>>> 
>>> 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 
>>> 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
 

Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-06 Thread Gary Gregory
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  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 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 :
> >
> > 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 
> > 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



Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-06 Thread Mark Struberg
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 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 :
> 
> 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 
> 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



Re: [LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-06 Thread Gary Gregory
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 
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
>
>


[LANG] EqualsBuilder#reflectionEquals feature brainstorming

2024-03-06 Thread Mark Struberg
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