RFR: JDK-8304439: Subscription based listeners [v7]

John Hendrikx jhendrikx at openjdk.org
Sun Jul 9 21:20:05 UTC 2023


On Sun, 9 Jul 2023 19:12:24 GMT, Jose Pereda <jpereda at openjdk.org> wrote:

>> John Hendrikx has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add newline at end of ConditionalBinding file
>
> modules/javafx.base/src/main/java/javafx/beans/Subscription.java line 77:
> 
>> 75:      */
>> 76:     default Subscription and(Subscription other) {
>> 77:         Objects.requireNonNull(other, "other cannot be null");
> 
> As per javadoc, this equivalent to `Subscription.combine(this, other)`, so then, couldn't you just call it instead of doing a different implementation?

I didn't quite see your point immediately, I thought you meant we don't need this method, but you meant why not do:

      default Subscription and(Subscription other) { return Subscription.combine(this, other); }

I agree this would work, and I don't mind it either way, but calling `combine` does incur a bit more overhead (when called with only a list of 2).  The reason I think `and` is a bit more efficient this way is that I think two variable captures vs a `List` with 2 elements will be slightly more efficient.  Not that I thought about it that long when implementing it, I just wanted to avoid creating the varargs array, and then the `List` if it isn't strictly needed.

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

PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1257543221


More information about the openjfx-dev mailing list