<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p>Hi,</p>
    <p>I'm impressed you found that compatibility report. :-)</p>
    <p>The most severe compatibility issues indeed occur if the
      keySet(), values(), and entrySet() methods are overridden to have
      a covariant return type. Issues will arise with subclasses that
      already override any of those methods, either with the return
      types from Map, or with covariant overrides of their own.</p>
    <p>It's true that bridge methods will be generated if this is done,
      and they take care of the most common compatibility issues.
      However, there is a compatibility matrix to be explored. My
      compatibility analysis considered old-binary, recompiled-source,
      and modified-source cases for both a library that has an at-risk
      subclass as well as an application that uses that library
      subclass. Some startling things emerged. What pushed me over the
      edge to decide against covariant overrides (in this part of the
      Sequenced Collections JEP) was that the behavior of a class could
      change silently upon recompilation, even if the source code wasn't
      changed.</p>
    <p>Aside from this issue, there's a more fundamental semantic issue
      with the object design. If I have some ObservableMap
      implementation, presumably it provides a keySet() implementation
      that's not observable. If ObservableMap is changed to have the
      covariant overrides, this effectively imposes a new requirement
      that existing implementations cannot possibly fulfill. Sure, you
      could tinker things around so that things appear to work in some
      cases, but the semantics of doing this are highly questionable.</p>
    <p>Now JavaFX could decide go ahead with this anyway, for a couple
      reasons. One might be, JavaFX doesn't care as much as the JDK
      about compatibility. It's (mostly) a different project, and it
      might have different compatibility constraints. You folks need to
      decide that. A second reason might be because there are
      vanishingly few or zero implementations of ObservableMap "in the
      wild," so any incompatibility would not cause any actual problems.
      This is difficult, but it's possible to get some information by
      doing source code searches. (Unfortunately there appear to be
      several libraries out there that have something called
      "ObservableMap" which will complicate the analysis.)</p>
    <p>The alternative is to add observableKeySet/Values/EntrySet()
      default methods instead of covariant overrides. This is safer, but
      it does add some clutter to the API.</p>
    <p>s'marks<br>
    </p>
    <div class="moz-cite-prefix">On 10/26/22 1:24 PM, Nir Lisker wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:CA+0ynh8rsUJwytqNr8o_1Bt_B+noJNbP9EMZdjorUpNc5H_qmw@mail.gmail.com">
      
      <div dir="ltr">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.
        <div><br>
          <div>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.</div>
          <div><br>
          </div>
          <div>I'd like to continue this discussion about the API side.
            I have already had some advances on the implementation.</div>
          <div>
            <div><br>
            </div>
            <div>[1] <a href="https://openjdk.org/jeps/431" moz-do-not-send="true" class="moz-txt-link-freetext">https://openjdk.org/jeps/431</a></div>
            <div>[2] <a href="https://bugs.openjdk.org/browse/JDK-8266572" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8266572</a></div>
          </div>
        </div>
      </div>
      <br>
      <div class="gmail_quote">
        <div dir="ltr" class="gmail_attr">On Tue, May 31, 2022 at 12:02
          AM Nir Lisker <<a href="mailto:nlisker@gmail.com" moz-do-not-send="true" class="moz-txt-link-freetext">nlisker@gmail.com</a>>
          wrote:<br>
        </div>
        <blockquote class="gmail_quote" style="margin:0px 0px 0px
          0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
          <div dir="ltr">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.</div>
          <br>
          <div class="gmail_quote">
            <div dir="ltr" class="gmail_attr">On Mon, May 30, 2022 at
              11:50 AM John Hendrikx <<a href="mailto:john.hendrikx@gmail.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">john.hendrikx@gmail.com</a>>
              wrote:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0px 0px 0px
              0.8ex;border-left:1px solid
              rgb(204,204,204);padding-left:1ex">Sorry, I misunderstood,
              I missed that the methods weren't already <br>
              defined in ObservableMap, so no existing signature is
              changed.<br>
              <br>
              --John<br>
              <br>
              On 30/05/2022 09:39, Tom Schindl wrote:<br>
              > Hi,<br>
              ><br>
              > Well the binary compat IMHO is not a problem. If your
              subtype <br>
              > overwrites the return type of a method the compiler
              will inserts a <br>
              > bridge method:<br>
              ><br>
              > Take this example<br>
              ><br>
              > package bla;<br>
              ><br>
              > import java.util.ArrayList;<br>
              > import java.util.Collection;<br>
              > import java.util.List;<br>
              ><br>
              > public class Test {<br>
              >     public interface IB {<br>
              >         public Collection<String> get();<br>
              >     }<br>
              ><br>
              >     public interface I extends IB {<br>
              >         public List<String> get();<br>
              >     }<br>
              ><br>
              >     public class C implements I {<br>
              >         public ArrayList<String> get() {<br>
              >             return new ArrayList<String>();<br>
              >         }<br>
              >     }<br>
              > }<br>
              ><br>
              > if you look at C with javap you'll notice<br>
              ><br>
              > Compiled from "Test.java"<br>
              > public class bla.Test$C implements bla.Test$I {<br>
              >   final bla.Test this$0;<br>
              >   public bla.Test$C(bla.Test);<br>
              >   public java.util.ArrayList<java.lang.String>
              get();<br>
              >   public java.util.Collection get();<br>
              >   public java.util.List get();<br>
              > }<br>
              ><br>
              ><br>
              > The problem is more that if someone directly
              implemented ObservableMap <br>
              > him/her self that it won't compile anymore. So it is
              a source <br>
              > incompatible change.<br>
              ><br>
              > Tom<br>
              ><br>
              > Am 30.05.22 um 08:58 schrieb John Hendrikx:<br>
              >> It's not binary compatible, as changing the
              return type results in a <br>
              >> new method that compiled code won't be able to
              find.<br>
              >><br>
              >> See also "change result type (including void)"
              here: <br>
              >> <a href="https://urldefense.com/v3/__https://wiki.eclipse.org/Evolving_Java-based_APIs_2*Evolving_API_interfaces_-_API_methods__;Iw!!ACWV5N9M2RV99hQ!Jly4lMnm2UZQsTVKLfmhN7g0AHwp2nlUj4H4a-IfCIp4tqJXElDbEbDsVRhkL6Sa7l097FoQn8_Pi9YS$" rel="noreferrer" target="_blank" moz-do-not-send="true">https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_interfaces_-_API_methods</a>
              <br>
              >><br>
              >><br>
              >> --John<br>
              >><br>
              >> On 30/05/2022 03:22, Nir Lisker wrote:<br>
              >>> Hi,<br>
              >>><br>
              >>> Picking up an old issue, JDK-8091393 [1], I
              went ahead and looked at <br>
              >>> the<br>
              >>> work needed to implement it.<br>
              >>><br>
              >>> keySet() and entrySet() can both be made to
              return ObservableSet rather<br>
              >>> easily. values() will probably require an
              ObservableCollection<E> type.<br>
              >>><br>
              >>> Before discussing these details, my question
              is: is it backwards <br>
              >>> compatible<br>
              >>> to require that these  methods now return a
              more refined type? I <br>
              >>> think that<br>
              >>> it will break implementations of
              ObservableMap out in the wild if it<br>
              >>> overrides these methods in Map. What is the
              assessment here?<br>
              >>><br>
              >>> <a href="https://bugs.openjdk.java.net/browse/JDK-8091393" rel="noreferrer" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.java.net/browse/JDK-8091393</a><br>
            </blockquote>
          </div>
        </blockquote>
      </div>
    </blockquote>
  </body>
</html>