RFR: 8277868: Use Comparable.compare() instead of surrogate code
Instead of something like long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1); we can use `return Long.compare(x, y);` All replacements are done with IDE. ------------- Commit messages: - 8277868: Use Comparable.compare() instead of surrogate code Changes: https://git.openjdk.java.net/jdk/pull/6575/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277868 Stats: 70 lines in 12 files changed: 2 ins; 44 del; 24 mod Patch: https://git.openjdk.java.net/jdk/pull/6575.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575 PR: https://git.openjdk.java.net/jdk/pull/6575
On Fri, 26 Nov 2021 12:46:59 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Changes to java.net look good. Please obtain approval from reviewers in the other areas before integrating. ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Fri, 26 Nov 2021 12:46:59 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 249:
247: 248: private static int sign(int num) { 249: return Integer.compare(num, 0);
=> Integer.signum(num) ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Sat, 27 Nov 2021 22:50:55 GMT, Michael Bien <duke@openjdk.java.net> wrote:
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Use Integer.signum() in BasicTableUI
src/java.desktop/share/classes/javax/swing/plaf/basic/BasicTableUI.java line 249:
247: 248: private static int sign(int num) { 249: return Integer.compare(num, 0);
=> Integer.signum(num)
Done! ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision: 8277868: Use Integer.signum() in BasicTableUI ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/6575/files - new: https://git.openjdk.java.net/jdk/pull/6575/files/8fa8242a..6744a562 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=00-01 Stats: 6 lines in 1 file changed: 0 ins; 4 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/6575.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575 PR: https://git.openjdk.java.net/jdk/pull/6575
On Mon, 29 Nov 2021 08:18:47 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Use Integer.signum() in BasicTableUI
The core-libs file changes look fine. A 'client' reviewer should take a look too. ------------- Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6575
On Mon, 29 Nov 2021 08:18:47 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Use Integer.signum() in BasicTableUI
src/java.desktop/share/classes/java/awt/geom/Line2D.java line 115:
113: */ 114: public double getX1() { 115: return x1;
What do these changes have to do with the subject of the PR ? src/java.desktop/share/classes/sun/java2d/Spans.java line 322:
320: float otherStart = otherSpan.getStart(); 321: 322: return Float.compare(mStart, otherStart);
We don't need the variable any more, do we ? ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Mon, 6 Dec 2021 17:46:22 GMT, Phil Race <prr@openjdk.org> wrote:
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Use Integer.signum() in BasicTableUI
src/java.desktop/share/classes/java/awt/geom/Line2D.java line 115:
113: */ 114: public double getX1() { 115: return x1;
What do these changes have to do with the subject of the PR ?
Just a tiny clean-up in on of affected files. Do you want this to be reverted? ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Tue, 7 Dec 2021 08:16:08 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
src/java.desktop/share/classes/java/awt/geom/Line2D.java line 115:
113: */ 114: public double getX1() { 115: return x1;
What do these changes have to do with the subject of the PR ?
Just a tiny clean-up in on of affected files. Do you want this to be reverted?
I believe it makes to preserve the cast: the fields are of type `float` and explicit cast hints there's a type conversion. ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Mon, 6 Dec 2021 17:48:37 GMT, Phil Race <prr@openjdk.org> wrote:
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Use Integer.signum() in BasicTableUI
src/java.desktop/share/classes/sun/java2d/Spans.java line 322:
320: float otherStart = otherSpan.getStart(); 321: 322: return Float.compare(mStart, otherStart);
We don't need the variable any more, do we ?
inlined ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision: 8277868: Inline local var ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/6575/files - new: https://git.openjdk.java.net/jdk/pull/6575/files/6744a562..1b3a5d4b Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=01-02 Stats: 3 lines in 1 file changed: 0 ins; 2 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/6575.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575 PR: https://git.openjdk.java.net/jdk/pull/6575
On Tue, 7 Dec 2021 08:28:47 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Inline local var
src/java.base/share/classes/java/util/Calendar.java line 3420:
3418: private int compareTo(long t) { 3419: long thisTime = getMillisOf(this); 3420: return Long.compare(thisTime, t);
Probably, in this case `thisTime` variable can also be dropped. src/java.base/share/classes/java/util/Date.java line 977:
975: long thisTime = getMillisOf(this); 976: long anotherTime = getMillisOf(anotherDate); 977: return Long.compare(thisTime, anotherTime);
Looks like local variables can also be dropped here as each value is used once. src/java.base/share/classes/java/util/UUID.java line 517:
515: return (this.mostSigBits < val.mostSigBits ? -1 : 516: (this.mostSigBits > val.mostSigBits ? 1 : 517: Long.compare(this.leastSigBits, val.leastSigBits)));
In this case Javadoc specifies only -1, 0 or 1 can be returned. `Long.compare` does not specify this but returns these values. Couldn't it cause any problems in the future if implementation of `Long.compare` is changed? Does it make sense to use `Long.compare` for `mostSigBits` too? int mostSigBits = Long.compare(this.mostSigBits, val.mostSigBits); return mostSigBits != 0 ? mostSigBits : Long.compare(this.leastSigBits, val.leastSigBits); ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
On Tue, 7 Dec 2021 12:01:27 GMT, Alexey Ivanov <aivanov@openjdk.org> wrote:
Сергей Цыпанов has updated the pull request incrementally with one additional commit since the last revision:
8277868: Inline local var
src/java.base/share/classes/java/util/Calendar.java line 3420:
3418: private int compareTo(long t) { 3419: long thisTime = getMillisOf(this); 3420: return Long.compare(thisTime, t);
Probably, in this case `thisTime` variable can also be dropped.
Inlined
src/java.base/share/classes/java/util/Date.java line 977:
975: long thisTime = getMillisOf(this); 976: long anotherTime = getMillisOf(anotherDate); 977: return Long.compare(thisTime, anotherTime);
Looks like local variables can also be dropped here as each value is used once.
Inlined ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
Сергей Цыпанов has updated the pull request incrementally with two additional commits since the last revision: - 8277868: Revert irrelevant changes - 8277868: Inline local vars ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/6575/files - new: https://git.openjdk.java.net/jdk/pull/6575/files/1b3a5d4b..bb06bf2f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6575&range=02-03 Stats: 9 lines in 3 files changed: 0 ins; 3 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/6575.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6575/head:pull/6575 PR: https://git.openjdk.java.net/jdk/pull/6575
On Fri, 26 Nov 2021 12:46:59 GMT, Сергей Цыпанов <duke@openjdk.java.net> wrote:
Instead of something like
long x; long y; return (x < y) ? -1 : ((x == y) ? 0 : 1);
we can use `return Long.compare(x, y);`
All replacements are done with IDE.
This pull request has now been integrated. Changeset: 20db7800 Author: Sergey Tsypanov <sergei.tsypanov@yandex.ru> Committer: Roger Riggs <rriggs@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/20db7800a657b311eeac504a2bbae4adbc20... Stats: 77 lines in 12 files changed: 2 ins; 54 del; 21 mod 8277868: Use Comparable.compare() instead of surrogate code Reviewed-by: rriggs, aivanov ------------- PR: https://git.openjdk.java.net/jdk/pull/6575
participants (6)
-
Alexey Ivanov
-
Daniel Fuchs
-
Michael Bien
-
Phil Race
-
Roger Riggs
-
Сергей Цыпанов