On Thu, 13 Jul 2023 21:15:47 GMT, John Hendrikx <jhendr...@openjdk.org> wrote:
>> I'm not entirely sure about this; the `Subscription` interface is pretty >> neutral in how it can be used, and does not have any direct link to >> observables at all; it's more of a token that you get that can be used to >> cancel a previous action, it doesn't actively do anything (unless you count >> the wrapper that it puts around the provided function). >> >> As it is, they don't even need to be associated with `Observable` or >> `ObservableValue` at all. Just as easily you could do: >> >> FileInputStream inputStream = .. ; >> Subscription combined = Subscription.combine( >> property.subscribe(this::invalidated), >> () -> inputStream.close() >> ); >> >> combined.subscribe(); // closes input stream and removes >> invalidations subscriber >> >> Although I think it is a good description, I'm not sure it belongs on this >> class. > > I put your changes (that we agree upon so far) in #1177 > > As I said I'm not sure about the description for the Subscription class, and > it's not included yet, and so I also haven't yet removed the duplicated texts > about GC-ability from the `subscribe` methods. I also now wonder if `Subscription` perhaps is in the wrong package (javafx.beans currently). Perhaps it should have been in javafx.util ? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/1069#discussion_r1263065227
