Scala Programming Language
  1. Scala Programming Language
  2. SI-8019

swing.Publisher calls PartialFunction listeners for undefined values

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Scala 2.10.3
    • Fix Version/s: Scala 2.10.4-RC1
    • Component/s: None
    • Labels:
      None

      Description

      Publisher has a list of listeners of the Reaction type (ie PartialFunction[Event, Unit]), but publishes events to all listeners, even if the Reaction is not defined for that value:

      https://github.com/scala/scala/blob/6ff3c3fd/src/swing/scala/swing/Publisher.scala#L47

      A Coursera thread with people affected by this issue:

      https://class.coursera.org/reactive-001/forum/thread?thread_id=1315

      will pull request shortly...

        Issue Links

          Activity

          Hide
          Roberto Tyley added a comment -

          Pull Request submitted with https://github.com/scala/scala/pull/3205

          Show
          Roberto Tyley added a comment - Pull Request submitted with https://github.com/scala/scala/pull/3205
          Hide
          Sciss added a comment -

          This is a possibly expensive fix. If you look at https://github.com/rtyley/scala/blob/4ef949da930c2797e46fb206ea541da54811d9ba/src/swing/scala/swing/Reactions.scala#L23 you see that `Reactions.Impl`'s `apply` again needs to call `isDefined` for each registered function, so now with standard reactions `isDefined` will be called twice. That's probably why `publish` took the shortcut.

          The better solution IMO would be to make `subscribe` package private.

          An immediate improvement could be to check the listener for `Reactions` in `publish`, and then shortcutting to `apply`.

          Show
          Sciss added a comment - This is a possibly expensive fix. If you look at https://github.com/rtyley/scala/blob/4ef949da930c2797e46fb206ea541da54811d9ba/src/swing/scala/swing/Reactions.scala#L23 you see that `Reactions.Impl`'s `apply` again needs to call `isDefined` for each registered function, so now with standard reactions `isDefined` will be called twice. That's probably why `publish` took the shortcut. The better solution IMO would be to make `subscribe` package private. An immediate improvement could be to check the listener for `Reactions` in `publish`, and then shortcutting to `apply`.

            People

            • Assignee:
              Roberto Tyley
              Reporter:
              Roberto Tyley
            • Votes:
              2 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development