Return-Path: Delivered-To: apmail-jakarta-commons-dev-archive@apache.org Received: (qmail 16506 invoked from network); 12 Sep 2002 09:20:07 -0000 Received: from unknown (HELO nagoya.betaversion.org) (192.18.49.131) by daedalus.apache.org with SMTP; 12 Sep 2002 09:20:07 -0000 Received: (qmail 7509 invoked by uid 97); 12 Sep 2002 09:20:51 -0000 Delivered-To: qmlist-jakarta-archive-commons-dev@jakarta.apache.org Received: (qmail 7449 invoked by uid 97); 12 Sep 2002 09:20:50 -0000 Mailing-List: contact commons-dev-help@jakarta.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Subscribe: List-Help: List-Post: List-Id: "Jakarta Commons Developers List" Reply-To: "Jakarta Commons Developers List" Delivered-To: mailing list commons-dev@jakarta.apache.org Received: (qmail 7431 invoked by uid 98); 12 Sep 2002 09:20:49 -0000 X-Antivirus: nagoya (v4218 created Aug 14 2002) Message-ID: <5811667.1031822411577.JavaMail.root@127.0.0.1> Date: Thu, 12 Sep 2002 10:20:11 +0100 (BST) From: scolebourne@btopenworld.com To: commons-dev@jakarta.apache.org Subject: Re: [lang] EqualsBuilder Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 7bit X-MAILER: talk21.com WAS v2 X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N X-Spam-Rating: daedalus.apache.org 1.6.2 0/1000/N > from: Steve Downey > > 1) I think that the reflection method should be static, and take the > > transient flag as a boolean parameter. The two techniques are distinct. > > this way forces it. > > return EqualsBuilder.reflectionEquals(obj1, obj2); > > return EqualsBuilder.reflectionEquals(obj1, obj2, boolean > > includeTransient); > > > I thought that too, at first, but the reflectionEquals (good name) method > needs to call append(), and append() is NOT side effect free. It sets the > isEquals flag on the EqualsBuilder object. > > Also, since the normal case requires an object, it's possible to call > reflectionEquals on an instance. If by some means reflectionEquals didn't use > the isEquals property, it could lead to subtle errors, since isEquals would > always be true. The solution is to create a new EqualsBuilder instance within the static reflectionEquals method. This fully keeps the separation, and avoids those subtle errors. Stephen ------------------------------ > > 2) The Object[] instanceof is coded in the Object[] method. I found that it > > needs to be coded in the Object method. For example: > > Object obj = new long[2]; > > is allowed, thus could be passed into the append(Object) method. > > Perverse, but possible. Why in hell aren't the [] classes derived > from something other than Object. Something that overrode equals, hashCode, > and so forth. > > I even ran into this problem in building the test cases. > > I looked into dealing with aliased arrays in reflectionEquals. It got very > ugly, very fast, so I didn't. It think it's outside the use case, anyway. > One of the inputs to reflectionEquals should be 'this', and 'this' can't be > an array. > > Changes made, tests added, and new files attached. > > And I retract my earliest comment about this being a conceptually lightweight > class. There are a lot of ugly corner cases that make writing an equals > method hard. > > > > > 3) The long[] etc. methods don't seem to check for nulls. > > > > > Stephen > > > > ----- Original Message ----- > > From: "Steve Downey" > > To: "Jakarta Commons Developers List" > > Sent: Wednesday, September 11, 2002 1:24 AM > > Subject: Re: [lang] EqualsBuilder > > > > > > Here's the updated version of EqualsBuilder. The multi-dim array stuff was > > particularly interesting. > > > > On Monday 09 September 2002 08:58 pm, Steve Downey wrote: > > > On Monday 09 September 2002 06:49 pm, Stephen Colebourne wrote: > > > > Comments: > > > > > > > > 1) I would use append as the method name. HashCode and ToString both > > > > use append - it seems to be a mini pattern. > > > > > > No problem. That's a simple search and replace. > > > > > > > 2) The double and float use an unusual comparison strategy. Was there a > > > > reason for this? > > > > > > Yes. Otherwise you can get bolixed up with NaN's, -0s, INFs and such. See > > > for example Effective Java Item 7, point 4. I'll add javadoc to explain > > > what's going on. > > > > > > > 3) Double and float could also have an allowed delta version. This > > > > would operate similarly to JUnit asserts. > > > > > > I disagree. In the context of equals(), you will violate the contract if > > > you use a tolerance. Two items that are equal will have different hash > > > codes. This is also part of the reason for point 2. > > > > > > It's critical that two objects that are equal behave identically. No > > > > matter > > > > > how small the tolerances, there are plenty of situations where the > > > > behavior > > > > > will diverge. Repeated multiplications by two is a trivial example. > > > > > > I'm looking at CompareToBuilder, also. The same problem applies there. If > > > you have a tolerance you end up with compareTo(a,b) == 0, compareTo(b,c) > > > > == > > > > > 0, and compareTo(a,c) != 0. > > > > > > > 4) The array handling has to be more complex to handle multi > > > > dimensional arrays. See ToStringBuilder. (Note to myself, I need to do > > > > this to HashCodeBuilder before a release...) > > > > > > Umm. I didn't run through it in depth, but I'm not seeing any handling of > > > multi-dimensional arrays in the ToStringBuilder that's in > > > commons-sandbox. I'm looking at version 1.2. It imports Array, Map and > > > Collection, but doesn't reference them. > > > OK, now I see it. In ToStringStyle, Object[] checks if the elements are > > > array types themselves. That's doable. And starts to add to the real > > > utility of the class, since it's tedious and errorprone to do correctly > > > by hand. > > > > > > > The reflection version definitely helps add usefulness as well. Similar > > > > code should be added to HashCodeBuilder and ToStringBuilder. It might > > > > be better with a 'don't include transient fields' flag. > > > > > > That's straightforward and useful. Transient fields are, almost by > > > definition, not part of the state of an object. [OK, sometimes they are, > > > but it's usually a bug.] The default should probably be to exclude them. > > > A flag could include them > > > > > > > So my vote is 1 with a little more work (as with all the builders at > > > > present) > > > > > > > > Stephen > > > > > > I'll make the changes and repost. > > > > > > > ----- Original Message ----- > > > > From: "Steve Downey" > > > > I saw this in the Todo's, and put it together. After building it, I'm > > > > not > > > > > > really sure it pulls it's own weight. Use is along these lines > > > > > > > > public boolean equals(Object o) { > > > > if (!o instanceof MyClass) { > > > > return false; > > > > } > > > > MyClass rhs = (MyClass) o; > > > > return new EqualsBuilder() > > > > .test(field1, rhs.field1) > > > > .test(field2, rhs.field2) > > > > .test(field3, rhs.field3) > > > > .isEquals(); > > > > > > > > } > > > > > > > > or > > > > > > > > public boolean equals(Object o) { > > > > return = new EqualsBuilder().reflectionTest(this,o); > > > > } > > > > > > > > the second form uses the setAccessible(true) hack, and so will throw > > > > exceptions under a security manager. OTOH, it's simple for testing > > > > code, which is where I seem to use equals most of the time. E.g. if a > > > > round trip through the database leaves an object equal to itself. > > > > > > > > IAC, I haven't done all the JavaDoc comments. I will if anyone thinks > > > > this would be useful for lang. > > > > > > ------------------------------------------------------------------------- > > > > > > >-- - ---- > > > > > > > > > -- > > > > > To unsubscribe, e-mail: > > > > > > > > > > > > > > > > > For additional commands, e-mail: > > > > > > > > > > > > --------------------------------------------------------------------------- > >- ---- > > > > > -- > > > To unsubscribe, e-mail: > > > > > > > > > For additional commands, e-mail: > > > > -- To unsubscribe, e-mail: For additional commands, e-mail: