Commons SCXML
  1. Commons SCXML
  2. SCXML-78

scxml-listener will not process in a defined order (problem: SCXMLExecutor.addListener based on Set)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.8
    • Fix Version/s: 0.9
    • Labels:
      None

      Description

      On 7/11/08, Daniel Schwager <Daniel.Schwager@dtnet.de> wrote:
      > Hi Rahul,
      >
      > i'm running in a problem: i add two listeners to my SCXMLExecutor instance
      > - the first one should run some business-methods using reflection (like you do in your watchclock-example)
      > - the second one should check the current state and maybe trigger some other fsm's
      >
      > The order of execution of the listeners is important, because
      > - FIRST I want to process the business-methods and
      > - SECOND I want to inform other compontents of reaching+processing of business-methods is done
      >
      > Your implementation of remember the listeners based on java.util.Set - the order in a
      > Set is not defined (because it's not an ordered element) - so, regardless of the sequence
      > Of adding my two listeners, your code executes the listeners in the wrong order (-:
      >
      > Did I miss something or is this a feature request ?
      >
      <snip/>

      In terms of the philosophy behind that piece of code, listeners
      shouldn't depend on the order in which they are invoked. Its possible
      to think about the scenario you describe as being only one listener,
      which you've broken up into two based on some usecase for convenience.

      However, in terms of implementation, its good to have predictable
      order. So while I still wouldn't recommend listeners that depend on
      order (earlier ones could have failed, its not a pipeline or a chain,
      various issues), lets make things predictable.

      Please open an improvement request in JIRA (and you can attach a patch
      [1] if you'd like, its probably not more than a line or two close to
      the bits you've identified below – we'd want a LinkedHashSet, not a
      Vector).

      -Rahul

      [1] http://commons.apache.org/patches.html

      > Regards
      > Danny
      >
      >
      >
      > SCXMLExecutor:
      > public void addListener(final SCXML scxml, final SCXMLListener listener)

      { > Object observable = scxml; > scInstance.getNotificationRegistry().addListener(observable, listener); > }

      >
      >
      > NotificationRegistry
      > private synchronized void fireOnEntry(final Object source,
      > final TransitionTarget state) {
      >
      > // ***** SET !!! not a Vector ..
      > Set entries = (Set) regs.get(source);
      > // ***** SET !!! not a Vector ..
      >
      > if (entries != null) {
      > for (Iterator iter = entries.iterator(); iter.hasNext()

      { > SCXMLListener lst = (SCXMLListener) iter.next(); > lst.onEntry(state); > }

      > }
      > }
      >
      > Viele Gruesse
      >
      > Daniel Schwager
      >

        Activity

        Hide
        Daniel Schwager added a comment -

        A patch agains NotificationRegistry.java replacing some Sets to LinkedHashSets

        The patch works for me (-:

        Thx
        Danny

        Show
        Daniel Schwager added a comment - A patch agains NotificationRegistry.java replacing some Sets to LinkedHashSets The patch works for me (-: Thx Danny
        Hide
        Rahul Akolkar added a comment -

        Thanks, setting fix version to next release, v0.9.

        Show
        Rahul Akolkar added a comment - Thanks, setting fix version to next release, v0.9.
        Hide
        Rahul Akolkar added a comment -

        Thanks for the patch. I've applied a variant (most of the casts aren't needed) and added you as a contributor in r680533:

        http://svn.apache.org/viewvc?rev=680533&view=rev

        Please try the latest code in SVN to verify the fix. Thanks.

        Show
        Rahul Akolkar added a comment - Thanks for the patch. I've applied a variant (most of the casts aren't needed) and added you as a contributor in r680533: http://svn.apache.org/viewvc?rev=680533&view=rev Please try the latest code in SVN to verify the fix. Thanks.
        Hide
        Rahul Akolkar added a comment -

        Closing since 0.9 is released.

        Show
        Rahul Akolkar added a comment - Closing since 0.9 is released.

          People

          • Assignee:
            Unassigned
            Reporter:
            Daniel Schwager
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development