RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods

classic Classic list List threaded Threaded
34 messages Options
12
Reply | Threaded
Open this post in threaded view
|

RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods

Joe Darcy-2
A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.

Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.

-------------

Commit messages:
 - Merge branch 'master' into 8261123
 - Initial changes for JDK-8261123.

Changes: https://git.openjdk.java.net/jdk/pull/2471/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261123
  Stats: 80 lines in 4 files changed: 38 ins; 14 del; 28 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2471.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2471/head:pull/2471

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods

Daniel Fuchs-2
On Tue, 9 Feb 2021 06:42:26 GMT, Joe Darcy <[hidden email]> wrote:

> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>
> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.

Small typos...

src/java.base/share/classes/java/lang/Object.java line 81:

> 79:      *     This integer need not remain consistent from one execution of an
> 80:      *     application to another execution of the same application.
> 81:      * <li>If two objects are equal according to the {@link equals(Object) equals}

I believe that should be: `{@link #equals(Object) equals}` (same a few line below).

src/java.base/share/classes/java/math/BigDecimal.java line 81:

> 79:  * modes of operation of the arithmetic defined in ANSI X3.274-1996
> 80:  * and ANSI X3.274-1996/AM 1-2000 (section 7.4).  Unlike those
> 81:  * standards, {@code BigDecimal} includes many rounding modes

Missing period at the end of the sentence (after "rounding modes")?

src/java.base/share/classes/java/math/BigDecimal.java line 91:

> 89:  * used in the result's representation.
> 90:
> 91:  * The different representations of same numerical value are called

Should that be:
The different representations of *a* same numerical value are called ...  - or maybe value is missing a 's'?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods

Joe Darcy-2
On Tue, 9 Feb 2021 18:51:33 GMT, Daniel Fuchs <[hidden email]> wrote:

>> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>>
>> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.
>
> src/java.base/share/classes/java/lang/Object.java line 81:
>
>> 79:      *     This integer need not remain consistent from one execution of an
>> 80:      *     application to another execution of the same application.
>> 81:      * <li>If two objects are equal according to the {@link equals(Object) equals}
>
> I believe that should be: `{@link #equals(Object) equals}` (same a few line below).

The `{@link equals(Object) equals}` idiom does result in the desired link in the generated javadoc.

> src/java.base/share/classes/java/math/BigDecimal.java line 81:
>
>> 79:  * modes of operation of the arithmetic defined in ANSI X3.274-1996
>> 80:  * and ANSI X3.274-1996/AM 1-2000 (section 7.4).  Unlike those
>> 81:  * standards, {@code BigDecimal} includes many rounding modes
>
> Missing period at the end of the sentence (after "rounding modes")?

Quite right; will fix in the next push. Thanks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v2]

Joe Darcy-2
In reply to this post by Joe Darcy-2
> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>
> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.

Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:

  Fix typos found in code review.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2471/files
  - new: https://git.openjdk.java.net/jdk/pull/2471/files/e6fde07a..f0d4d2b7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=00-01

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2471.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2471/head:pull/2471

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v2]

Joe Darcy-2
In reply to this post by Daniel Fuchs-2
On Tue, 9 Feb 2021 18:55:52 GMT, Daniel Fuchs <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos found in code review.
>
> src/java.base/share/classes/java/math/BigDecimal.java line 91:
>
>> 89:  * used in the result's representation.
>> 90:
>> 91:  * The different representations of same numerical value are called
>
> Should that be:
> The different representations of *a* same numerical value are called ...  - or maybe value is missing a 's'?

Missing "the" -- will fix; thanks.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v2]

Brian Burkhalter-3
In reply to this post by Joe Darcy-2
On Tue, 9 Feb 2021 20:27:52 GMT, Joe Darcy <[hidden email]> wrote:

>> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>>
>> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.
>
> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix typos found in code review.

src/java.base/share/classes/java/lang/Object.java line 148:

> 146:      * relation, each equivalence class only has a single element.
> 147:      *
> 148:      * @apiNoted

`s/apiNoted/apiNote/`

src/java.base/share/classes/java/lang/Object.java line 198:

> 196:      * need to be modified.
> 197:      *
> 198:      * @implspec

`s/implspec/implSpec/`

src/java.base/share/classes/java/math/BigDecimal.java line 3062:

> 3060:
> 3061:      * @apiNote
> 3062:      * Note: this class has a natural ordering that is inconsistent with equals.

Is `Note: ` really needed?

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Joe Darcy-2
> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>
> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.

Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:

  Fix typos in javadoc tags found during review.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2471/files
  - new: https://git.openjdk.java.net/jdk/pull/2471/files/f0d4d2b7..2a8dd8ce

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2471.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2471/head:pull/2471

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Brian Burkhalter-3
On Tue, 9 Feb 2021 19:41:25 GMT, Brian Burkhalter <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/lang/Object.java line 148:
>
>> 146:      * relation, each equivalence class only has a single element.
>> 147:      *
>> 148:      * @apiNoted
>
> `s/apiNoted/apiNote/`

Fixed. I did run a successful docs builds before sending out the change for review and I'm surprised the unrecognized tags didn't trigger a docs build failure. thanks.

> src/java.base/share/classes/java/math/BigDecimal.java line 3062:
>
>> 3060:
>> 3061:      * @apiNote
>> 3062:      * Note: this class has a natural ordering that is inconsistent with equals.
>
> Is `Note: ` really needed?

This is the exact text recommended in java.lang.Comparable when a type's natural ordering is inconsistent with equals. The statement to that effect at the top of BigDecimal didn't use that exact wording

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Brian Burkhalter-3
In reply to this post by Joe Darcy-2
On Wed, 10 Feb 2021 01:49:55 GMT, Joe Darcy <[hidden email]> wrote:

>> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>>
>> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.
>
> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix typos in javadoc tags found during review.

Marked as reviewed by bpb (Reviewer).

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Brian Burkhalter-3
In reply to this post by Joe Darcy-2
On Wed, 10 Feb 2021 01:55:24 GMT, Joe Darcy <[hidden email]> wrote:

>> src/java.base/share/classes/java/math/BigDecimal.java line 3062:
>>
>>> 3060:
>>> 3061:      * @apiNote
>>> 3062:      * Note: this class has a natural ordering that is inconsistent with equals.
>>
>> Is `Note: ` really needed?
>
> This is the exact text recommended in java.lang.Comparable when a type's natural ordering is inconsistent with equals. The statement to that effect at the top of BigDecimal didn't use that exact wording

Okay, I see.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Stuart Marks-2
In reply to this post by Joe Darcy-2
On Wed, 10 Feb 2021 01:49:55 GMT, Joe Darcy <[hidden email]> wrote:

>> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>>
>> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.
>
> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>
>   Fix typos in javadoc tags found during review.

Overall very nice. Mostly my comments are wordsmithing. Two issues are worth considering; see detailed comments inline.

1) Do we want a general statement on the stability of the value returned by toString()?

2) Add rationale and example for why BigDecimal's natural ordering is inconsistent with equals.

src/java.base/share/classes/java/lang/Comparable.java line 68:

> 66:  * orderings that are consistent with equals.  One exception is
> 67:  * {@link java.math.BigDecimal}, whose {@linkplain java.math.BigDecimal#compareTo natural ordering} equates
> 68:  * {@code BigDecimal} objects with equal numerical values and different representations

Nothing here talks about the representation of BigDecimal. I think it would be preferable to say that in BigDecimal, the `equals()` method considers 4.0 and 4.00 unequal because they have different precisions; however, `compareTo()` considers them equal because it considers only their numerical values.

src/java.base/share/classes/java/lang/Comparable.java line 90:

> 88:  * of the {@code equals} method and the equivalence classes defined by
> 89:  * the quotient of the {@code compareTo} method are the same.
> 90:  *

I think in both cases it should be "the equivalence class defined by...." That is, "equivalence class" should be singular. But there are two of them, so the sentence still properly concludes "... are the same."

src/java.base/share/classes/java/lang/Comparable.java line 110:

> 108:      * {@link Integer#signum signum}{@code (x.compareTo(y)) == -signum(y.compareTo(x))}
> 109:      * for all {@code x} and {@code y}.  (This
> 110:      * implies that {@code x.compareTo(y)} must throw an exception iff

I'd suggest replacing "iff" with "if and only if" because it looks like a typo, and writing out the long form emphasizes the bidirectional nature of the implication.

src/java.base/share/classes/java/lang/Object.java line 135:

> 133:      * into <i>equivalence classes</i>; all the members of an
> 134:      * equivalence class are equal to each other. The equal objects
> 135:      * are substitutable for each other, at least for some purposes.

Since the preceding sentence mentions the members of an equivalence class, it might read better to say "Members are substitutable..." or possibly "Members of an equivalence class are substitutable...."

src/java.base/share/classes/java/lang/Object.java line 149:

> 147:      *
> 148:      * @apiNote
> 149:      * Note that it is generally necessary to override the {@link hashCode hashCode}

The `@apiNote` introduces the paragraph with a subhead "API Note:" so it's a bit redundant to say "Note that...." Maybe just start with "It is generally necessary...."

src/java.base/share/classes/java/lang/Object.java line 236:

> 234:      * be a concise but informative representation that is easy for a
> 235:      * person to read.
> 236:      * It is recommended that all subclasses override this method.

Do we want to have a general note here or somewhere that the exact format of the result of `toString()` is generally not stable, and that it is subject to change without notice?

src/java.base/share/classes/java/math/BigDecimal.java line 97:

> 95:  * contrast, the {@link equals equals} method requires both the
> 96:  * numerical value and representation to be the same for equality to
> 97:  * hold.

Note, discussing "representation" is ok here since the context is discussing the representation of BigDecimal (in contrast to the text in Comparable).

src/java.base/share/classes/java/util/Comparator.java line 95:

> 93:  * equals, the equivalence classes defined by the equivalence relation
> 94:  * of the {@code equals} method and the equivalence classes defined by
> 95:  * the quotient of the {@code compare} method are the same.

As before, suggest "equivalence class" be singular in both cases.

src/java.base/share/classes/java/util/Comparator.java line 159:

> 157:      * and it imposes the same ordering as this comparator.  Thus,
> 158:      * {@code comp1.equals(comp2)} implies that {@code signum(comp1.compare(o1,
> 159:      * o2))==signum(comp2.compare(o1, o2))} for every object reference

Maybe make "signum" be a link here, similar to other locations where it's used.

-------------

Marked as reviewed by smarks (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Stuart Marks-2
In reply to this post by Brian Burkhalter-3
On Wed, 10 Feb 2021 02:55:14 GMT, Brian Burkhalter <[hidden email]> wrote:

>> This is the exact text recommended in java.lang.Comparable when a type's natural ordering is inconsistent with equals. The statement to that effect at the top of BigDecimal didn't use that exact wording
>
> Okay, I see.

The note itself should be here, but it's demarcated with an `@apiNote` tag. This introduces a subhead "API Note:" in the rendered javadoc. Thus, it's not necessary to start the text with "Note:".

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Stuart Marks-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:24:40 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/math/BigDecimal.java line 97:
>
>> 95:  * contrast, the {@link equals equals} method requires both the
>> 96:  * numerical value and representation to be the same for equality to
>> 97:  * hold.
>
> Note, discussing "representation" is ok here since the context is discussing the representation of BigDecimal (in contrast to the text in Comparable).

It might be reasonable to add a bit of rationale here and give an example. I think the reason that members of the same cohort might not be considered `equals()` to one another is that they are not substitutable. For example, consider 2.0 and 2.00. They are members of the same cohort, because

    new BigDecimal("2.0").compareTo(new BigDecimal("2.00")) == 0

is true. However,

    new BigDecimal("2.0").equals(new BigDecimal("2.00"))

is false. They aren't considered `equals()` because they aren't substitutable, since using them in an arithmetic expression can give different results. For example:

    new BigDecimal("2.0").divide(new BigDecimal(3), RoundingMode.HALF_UP)
    ==> 0.7

    new BigDecimal("2.00").divide(new BigDecimal(3), RoundingMode.HALF_UP)
    ==> 0.67

I think that's the right rationale, and a reasonable example to illustrate it. Edit to taste. I think it would be good to have material like this, though, because people's immediate reaction to BD being inconsistent with equals is "well that's wrong." Well, no, it's right, and I think this is the reason.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:29:11 GMT, Stuart Marks <[hidden email]> wrote:

>> Okay, I see.
>
> The note itself should be here, but it's demarcated with an `@apiNote` tag. This introduces a subhead "API Note:" in the rendered javadoc. Thus, it's not necessary to start the text with "Note:".

My thinking here was to include the exact string "Note: this class has a natural ordering that is inconsistent with equals." in the text of BigDecimal since that is the phrase java.lang.Comparable recommends. This should improve grep-ability, etc. for such classes in the JDK. The class-level summary has a semantically equivalent statement using different wording, which left the compareTo method as a place to introduce the exact phrase. I use an @apiNote both because it is an informative statement and to prevent the text from necessarily being applied to BigDecimal subclasses. BigDecimal is not final and people can and sometimes do subclass it and those subclasses, in principle, could have a (different) natural order that was consistent with equals.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:14:08 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/lang/Comparable.java line 110:
>
>> 108:      * {@link Integer#signum signum}{@code (x.compareTo(y)) == -signum(y.compareTo(x))}
>> 109:      * for all {@code x} and {@code y}.  (This
>> 110:      * implies that {@code x.compareTo(y)} must throw an exception iff
>
> I'd suggest replacing "iff" with "if and only if" because it looks like a typo, and writing out the long form emphasizes the bidirectional nature of the implication.

Sure; most of the terminology in the JDK docs aren't very "math-y".

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:19:29 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/lang/Object.java line 149:
>
>> 147:      *
>> 148:      * @apiNote
>> 149:      * Note that it is generally necessary to override the {@link hashCode hashCode}
>
> The `@apiNote` introduces the paragraph with a subhead "API Note:" so it's a bit redundant to say "Note that...." Maybe just start with "It is generally necessary...."

Agree.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:37:34 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/util/Comparator.java line 159:
>
>> 157:      * and it imposes the same ordering as this comparator.  Thus,
>> 158:      * {@code comp1.equals(comp2)} implies that {@code signum(comp1.compare(o1,
>> 159:      * o2))==signum(comp2.compare(o1, o2))} for every object reference
>
> Maybe make "signum" be a link here, similar to other locations where it's used.

Okay -- I didn't want to go overboard making all the signum references links to the method, but I can change the first occurrence in Comparator.equals to a link.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v4]

Joe Darcy-2
In reply to this post by Joe Darcy-2
> A follow-up of sorts to JDK-8257086, this change aims to improve the discussion of the relationship between Object.equals and compareTo and compare methods. The not-consistent-with-equals natural ordering of BigDecimal get more explication too. While updating Object, I added some uses of apiNote and implSpec, as appropriate. While a signum method did not exist when Comparable was added, it has existed for many years so I removed obsolete text that defined a "sgn" function to compute signum.
>
> Once the changes are agreed to, I'll reflow paragraphs and update the copyright years.

Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:

  Respond to review feedback.

-------------

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/2471/files
  - new: https://git.openjdk.java.net/jdk/pull/2471/files/2a8dd8ce..a7f1b28b

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2471&range=02-03

  Stats: 3 lines in 3 files changed: 0 ins; 0 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2471.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2471/head:pull/2471

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:09:16 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/lang/Comparable.java line 68:
>
>> 66:  * orderings that are consistent with equals.  One exception is
>> 67:  * {@link java.math.BigDecimal}, whose {@linkplain java.math.BigDecimal#compareTo natural ordering} equates
>> 68:  * {@code BigDecimal} objects with equal numerical values and different representations
>
> Nothing here talks about the representation of BigDecimal. I think it would be preferable to say that in BigDecimal, the `equals()` method considers 4.0 and 4.00 unequal because they have different precisions; however, `compareTo()` considers them equal because it considers only their numerical values.

In BigDecimal, I'll add a sentence:

" The results of methods like {@link scale} and {@link unscaledValue} will differ for numerically equal values with different representations."

Talking about "precision" and BigDecimal is more appropriate for pre-JSR 13 BigDecimal since values like zero can have the same precision (1 digit) but many different scales.

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8261123: Augment discussion of equivalence classes in Object.equals and comparison methods [v3]

Joe Darcy-2
In reply to this post by Stuart Marks-2
On Thu, 11 Feb 2021 04:12:06 GMT, Stuart Marks <[hidden email]> wrote:

>> Joe Darcy has updated the pull request incrementally with one additional commit since the last revision:
>>
>>   Fix typos in javadoc tags found during review.
>
> src/java.base/share/classes/java/lang/Comparable.java line 90:
>
>> 88:  * of the {@code equals} method and the equivalence classes defined by
>> 89:  * the quotient of the {@code compareTo} method are the same.
>> 90:  *
>
> I think in both cases it should be "the equivalence class defined by...." That is, "equivalence class" should be singular. But there are two of them, so the sentence still properly concludes "... are the same."

An equivalence relation defines a *set* of equivalence classes which partition the objects the relation operates on. For example, the "same signum value" equivalence relation on integers has three equivalence classes :
1) negative nonzero numbers (corresponding to signum == -1)
2) zero (corresponding to signum = 0)
3) positive nonzero numbers (corresponding to signum ==1)

-------------

PR: https://git.openjdk.java.net/jdk/pull/2471
12