glazedlists
  1. glazedlists
  2. GLAZEDLISTS-481

SequenceDependenciesEventPublisher.clearRelatedListener() swapped parameters

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Incomplete
    • Affects Version/s: current
    • Fix Version/s: milestone 1
    • Component/s: core
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      481

      Description

      Apparently SequenceDependenciesEventPublisher.clearRelatedListener() calls
      removeListener with wrong swapped parameters:

      public void clearRelatedListener(Object subject, Object relatedListener)

      { removeListener(relatedListener, subject); }

        Activity

        Hide
        brands added a comment -

        Here is the code for setRelatedListener and clearRelatedListener:

        public void setRelatedListener(Object subject, Object relatedListener)

        { // force the dependency by adding a listener that just doesn't // do anything. This will make sure that subject is always after // related listener in the dependencies graph addListener(relatedListener, subject, NoOpEventFormat.INSTANCE); }

        /**

        {@inheritDoc}

        */
        public void clearRelatedListener(Object subject, Object relatedListener)

        { removeListener(relatedListener, subject); }

        So the code looks correct to me, as the addListener and removeListener methods
        are called consistently with the same parameter ordering.

        It's important that you call setRelatedListener and clearRelatedListener with
        the same consistent parameter ordering from your code.

        If you still think there is an error, please provide a test case to demonstrate
        the issue.

        Show
        brands added a comment - Here is the code for setRelatedListener and clearRelatedListener: public void setRelatedListener(Object subject, Object relatedListener) { // force the dependency by adding a listener that just doesn't // do anything. This will make sure that subject is always after // related listener in the dependencies graph addListener(relatedListener, subject, NoOpEventFormat.INSTANCE); } /** {@inheritDoc} */ public void clearRelatedListener(Object subject, Object relatedListener) { removeListener(relatedListener, subject); } So the code looks correct to me, as the addListener and removeListener methods are called consistently with the same parameter ordering. It's important that you call setRelatedListener and clearRelatedListener with the same consistent parameter ordering from your code. If you still think there is an error, please provide a test case to demonstrate the issue.
        Hide
        brunomedeiros added a comment -

        I meant the arguments for the removeListener call seem to be swapped. If we look
        at the removeListener() signature it's:

        public synchronized void removeListener(Object subject, Object listener) {

        but in clearRelatedListener() it is called like this:

        removeListener(relatedListener, subject);

        So the arguments are either swapped or I am misunderstanting and subjects are
        interchangeable as listeners, and vice-versa.

        Show
        brunomedeiros added a comment - I meant the arguments for the removeListener call seem to be swapped. If we look at the removeListener() signature it's: public synchronized void removeListener(Object subject, Object listener) { but in clearRelatedListener() it is called like this: removeListener(relatedListener, subject); So the arguments are either swapped or I am misunderstanting and subjects are interchangeable as listeners, and vice-versa.
        Hide
        brands added a comment -

        In this case the swapped parameters are intentional.

        The javadoc of the setRelatedListener-method states:
        "Attach the specified subject to the specified listener, so that the
        listener's dependencies are satisfied before the subject is notified."

        The implementation achieves this by registering the subject (param 1) as an
        artificial listener of the passed in listener (param 2) which now becomes the
        subject!
        So the roles are really swapped in this case.

        The clearRelatedListener-method has the same parameter ordering as the
        setRelatedListener-method.

        I admit, this part of the API is difficult to understand. I always have to think
        about the correct parameters twice, too

        Show
        brands added a comment - In this case the swapped parameters are intentional. The javadoc of the setRelatedListener-method states: "Attach the specified subject to the specified listener, so that the listener's dependencies are satisfied before the subject is notified." The implementation achieves this by registering the subject (param 1) as an artificial listener of the passed in listener (param 2) which now becomes the subject! So the roles are really swapped in this case. The clearRelatedListener-method has the same parameter ordering as the setRelatedListener-method. I admit, this part of the API is difficult to understand. I always have to think about the correct parameters twice, too
        Hide
        brunomedeiros added a comment -

        Ah, ok, my misunderstanding then. _'

        Show
        brunomedeiros added a comment - Ah, ok, my misunderstanding then. _ '
        Hide
        brands added a comment -

        not a bug ...

        Show
        brands added a comment - not a bug ...

          People

          • Assignee:
            jessewilson
            Reporter:
            brunomedeiros
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: