I'm CC'ing Stuart Marks who has recently dealt with a similar issue when
working on Sequenced Collections [1], and wrote a compatibility report [2]
that includes an item about covariant overrides ("Covariant Overrides of
`SequencedMap` View Collection Methods"), which is similar to what is
discussed here. I contacted him off list to get his insights into the risks
involved here.To recap, ObservableMap inherits keySet(), entrySet() and values() from Map, which return the standard Set and Collection interfaces. ObservableMap should provide ObservableSet and perhaps the not-yet-existing ObservableCollection. There are 2 options here: one is to add additional default methods to ObservableMap that return observable collection, the second is to override the methods inherited from Map and change the return value. The latter has some backwards compatibility issues. It comes down to implementations of ObservableMap in the wild. I have yet to see any, personally. JavaFX does not itself expose any of its implementations, as ObservableMaps are obtained through FXCollections static methods. I'd like to continue this discussion about the API side. I have already had some advances on the implementation. [1] https://openjdk.org/jeps/431 [2] https://bugs.openjdk.org/browse/JDK-8266572 On Tue, May 31, 2022 at 12:02 AM Nir Lisker <nlis...@gmail.com> wrote: > Then maybe a solution would be around adding new methods like > observableKeySet(). These will need to be default methods, and the > implementation could test if keySet() already returns an ObservableSet, in > which case it returns it, and if not it wraps the Set in an > ObservableSetWrapper or something like that. > > On Mon, May 30, 2022 at 11:50 AM John Hendrikx <john.hendr...@gmail.com> > wrote: > >> Sorry, I misunderstood, I missed that the methods weren't already >> defined in ObservableMap, so no existing signature is changed. >> >> --John >> >> On 30/05/2022 09:39, Tom Schindl wrote: >> > Hi, >> > >> > Well the binary compat IMHO is not a problem. If your subtype >> > overwrites the return type of a method the compiler will inserts a >> > bridge method: >> > >> > Take this example >> > >> > package bla; >> > >> > import java.util.ArrayList; >> > import java.util.Collection; >> > import java.util.List; >> > >> > public class Test { >> > public interface IB { >> > public Collection<String> get(); >> > } >> > >> > public interface I extends IB { >> > public List<String> get(); >> > } >> > >> > public class C implements I { >> > public ArrayList<String> get() { >> > return new ArrayList<String>(); >> > } >> > } >> > } >> > >> > if you look at C with javap you'll notice >> > >> > Compiled from "Test.java" >> > public class bla.Test$C implements bla.Test$I { >> > final bla.Test this$0; >> > public bla.Test$C(bla.Test); >> > public java.util.ArrayList<java.lang.String> get(); >> > public java.util.Collection get(); >> > public java.util.List get(); >> > } >> > >> > >> > The problem is more that if someone directly implemented ObservableMap >> > him/her self that it won't compile anymore. So it is a source >> > incompatible change. >> > >> > Tom >> > >> > Am 30.05.22 um 08:58 schrieb John Hendrikx: >> >> It's not binary compatible, as changing the return type results in a >> >> new method that compiled code won't be able to find. >> >> >> >> See also "change result type (including void)" here: >> >> >> https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods >> >> >> >> >> >> --John >> >> >> >> On 30/05/2022 03:22, Nir Lisker wrote: >> >>> Hi, >> >>> >> >>> Picking up an old issue, JDK-8091393 [1], I went ahead and looked at >> >>> the >> >>> work needed to implement it. >> >>> >> >>> keySet() and entrySet() can both be made to return ObservableSet >> rather >> >>> easily. values() will probably require an ObservableCollection<E> >> type. >> >>> >> >>> Before discussing these details, my question is: is it backwards >> >>> compatible >> >>> to require that these methods now return a more refined type? I >> >>> think that >> >>> it will break implementations of ObservableMap out in the wild if it >> >>> overrides these methods in Map. What is the assessment here? >> >>> >> >>> https://bugs.openjdk.java.net/browse/JDK-8091393 >> >
