Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8263233: Refactored equals method further Good to see the new feature being used. It definitely enhances readability overall. - Marked as reviewed by michaelm (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 20:52:19 GMT, Mark Sheppard wrote: >> The disadvantage is that it would add another level of braces. > > maybe ... I find it would provide an easier flow to code ... an "easy on the > eye" call flow linkage - the variable is declared and then used instead > of the disconnect of throwing an exception or non immediate use of the > variable. > > but sure, isn't code somewhat idiosyncratic!! The first few times I looked at this pattern, I did stumble over it somewhat. But since then, I find it natural enough and easy to follow. Maybe the concern is transient in nature, until the instanceof pattern matching feature is more widely used. - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8263233: Refactored equals method further Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 20:26:19 GMT, Daniel Fuchs wrote: >> src/java.base/share/classes/java/net/NetMulticastSocket.java line 219: >> >>> 217: if (addr == null) >>> 218: addr = new InetSocketAddress(0); >>> 219: if (!(addr instanceof InetSocketAddress epoint)) >> >> in the context of this type of change the negative logic is a little obtuse >> or disjoint in the new style variable declaration, and its subsequent use. >> I'd find it a more natural flow and easier to read with the instanceof >> pattern prefacing a block in which the variable is used >> if (addr instanceof InetSocketAddress epoint) { >> // a block which is using the epoint variable >>if (epoint.isUnresolved) >> throw new SocketException("Unresolved address"); >>InetAddress iaddr = epoint.getAddress; >> etc, etc ... >> } else { >>throw new IllegalArgumentException("Unsupported address type!"); >> } > > The disadvantage is that it would add another level of braces. maybe ... I find it would provide an easier flow to code ... an "easy on the eye" call flow linkage - the variable is declared and then used instead of the disconnect of throwing an exception or non immediate use of the variable. but sure, isn't code somewhat idiosyncratic!! - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8263233: Refactored equals method further Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 20:07:14 GMT, Mark Sheppard wrote: >> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8263233: Refactored equals method further > > src/java.base/share/classes/java/net/NetMulticastSocket.java line 219: > >> 217: if (addr == null) >> 218: addr = new InetSocketAddress(0); >> 219: if (!(addr instanceof InetSocketAddress epoint)) > > in the context of this type of change the negative logic is a little obtuse > or disjoint in the new style variable declaration, and its subsequent use. > I'd find it a more natural flow and easier to read with the instanceof > pattern prefacing a block in which the variable is used > if (addr instanceof InetSocketAddress epoint) { > // a block which is using the epoint variable >if (epoint.isUnresolved) > throw new SocketException("Unresolved address"); >InetAddress iaddr = epoint.getAddress; > etc, etc ... > } else { >throw new IllegalArgumentException("Unsupported address type!"); > } The disadvantage is that it would add another level of braces. - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 18:22:17 GMT, Chris Hegarty wrote: >> Patrick Concannon has updated the pull request incrementally with one >> additional commit since the last revision: >> >> 8263233: Refactored equals method further > > src/java.base/share/classes/java/net/InterfaceAddress.java line 108: > >> 106: if (Objects.equals(address, cmp.address) && >> 107: Objects.equals(broadcast, cmp.broadcast) && >> 108: maskLength == cmp.maskLength) > > Apologies Patrick, building on Michael's suggestion, then you'd end up at: > > public boolean equals(Object obj) { > return obj instanceof InterfaceAddress cmp >&& Objects.equals(address, cmp.address) >&& Objects.equals(broadcast, cmp.broadcast) >&& maskLength == cmp.maskLength; > } My mistake. I've fixed that now. See 27a58926be98ef0137244183ac0ee21c88967a1c - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
On Tue, 9 Mar 2021 19:56:25 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.net` >> and `java.nio` packages to make use of the `instanceof` pattern variable? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8263233: Refactored equals method further src/java.base/share/classes/java/net/NetMulticastSocket.java line 219: > 217: if (addr == null) > 218: addr = new InetSocketAddress(0); > 219: if (!(addr instanceof InetSocketAddress epoint)) in the context of this type of change the negative logic is a little obtuse or disjoint in the new style variable declaration, and its subsequent use. I'd find it a more natural flow and easier to read with the instance pattern prefacing a block in which the variable is used if (addr instanced InetSocketAddress epoint) { // a block which is using the epoint variable if (epoint.isUnresolved) throw new SocketException("Unresolved address"); InetAddress iaddr = epoint.getAddress; etc, etc ... } else { throw new IllegalArgumentException("Unsupported address type!"); } - PR: https://git.openjdk.java.net/jdk/pull/2890
Re: RFR: 8263233: Update java.net and java.nio to use instanceof pattern variable [v3]
> Hi, > > Could someone please review my code for updating the code in the `java.net` > and `java.nio` packages to make use of the `instanceof` pattern variable? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8263233: Refactored equals method further - Changes: - all: https://git.openjdk.java.net/jdk/pull/2890/files - new: https://git.openjdk.java.net/jdk/pull/2890/files/eca90955..27a58926 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2890&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2890&range=01-02 Stats: 6 lines in 1 file changed: 0 ins; 2 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/2890.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2890/head:pull/2890 PR: https://git.openjdk.java.net/jdk/pull/2890