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 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=2890=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2890=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
