Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-22 Thread Kevin Rushforth
On Tue, 19 Jul 2022 04:17:18 GMT, Nir Lisker wrote: >> you bring a good point, Kevin, thank you! > > I would use `Boolean.hashCode(relative);` for a `boolean`. > > Kevin, I checked Effective Java 3rd Edition and it also says to use 0 (or > some other constant) for `null`. Using 0 for a null co

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-18 Thread Nir Lisker
On Mon, 11 Jul 2022 22:40:29 GMT, Andy Goryachev wrote: >> In order to reduce collisions, the hash of each component is typically added >> to `h * 31` even when that hash is 0, whereas you skip the `h = 31 * h` in >> the case of null. It might not be a problem in practice, since value and >> o

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-11 Thread Andy Goryachev
On Mon, 11 Jul 2022 21:12:02 GMT, Kevin Rushforth wrote: >> Ah, I didn't realize you checked how this is optimized by the JIT. > > In order to reduce collisions, the hash of each component is typically added > to `h * 31` even when that hash is 0, whereas you skip the `h = 31 * h` in > the case

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-11 Thread Kevin Rushforth
On Mon, 11 Jul 2022 18:56:36 GMT, John Hendrikx wrote: >> @hjohn: yes, but at a price: Object.hash(Object ...) incurs overhead by >> creating a temporary Object[] + boxing of a Boolean. > > Ah, I didn't realize you checked how this is optimized by the JIT. In order to reduce collisions, the has

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-11 Thread John Hendrikx
On Mon, 11 Jul 2022 15:19:30 GMT, Andy Goryachev wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/css/CalculatedValue.java >> line 101: >> >>> 99: } >>> 100: return h; >>> 101: } >> >> Just an example, but wouldn't: `Objects.hash(relative, origin, value)` here

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-11 Thread Andy Goryachev
On Fri, 8 Jul 2022 23:45:58 GMT, John Hendrikx wrote: >> Andy Goryachev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8289389: toExternalForm() > > modules/javafx.graphics/src/main/java/com/sun/javafx/css/CalculatedValue.java > line 1

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-09 Thread Kevin Rushforth
On Sat, 9 Jul 2022 01:28:20 GMT, Nir Lisker wrote: >> modules/javafx.graphics/src/main/java/com/sun/javafx/css/CalculatedValue.java >> line 101: >> >>> 99: } >>> 100: return h; >>> 101: } >> >> Just an example, but wouldn't: `Objects.hash(relative, origin, value)` here >>

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-08 Thread Nir Lisker
On Fri, 8 Jul 2022 23:45:58 GMT, John Hendrikx wrote: >> Andy Goryachev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8289389: toExternalForm() > > modules/javafx.graphics/src/main/java/com/sun/javafx/css/CalculatedValue.java > line 1

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-08 Thread John Hendrikx
On Fri, 8 Jul 2022 21:53:50 GMT, Andy Goryachev wrote: >> - added missing hashCode() methods > > Andy Goryachev has updated the pull request incrementally with one additional > commit since the last revision: > > 8289389: toExternalForm() Just a general note, shouldn't most of these use `Obj

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-08 Thread Andy Goryachev
On Fri, 8 Jul 2022 21:41:29 GMT, Kevin Rushforth wrote: >> Andy Goryachev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8289389: toExternalForm() > > modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java line 2329: > >> 2327:

Re: RFR: 8289389: Fix warnings: type should also implement hashCode() since it overrides Object.equals() [v2]

2022-07-08 Thread Andy Goryachev
> - added missing hashCode() methods Andy Goryachev has updated the pull request incrementally with one additional commit since the last revision: 8289389: toExternalForm() - Changes: - all: https://git.openjdk.org/jfx/pull/821/files - new: https://git.openjdk.org/jfx/pull/82