Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.1
    • Fix Version/s: 2.0.4
    • Component/s: core
    • Labels:
      None
    • Environment:

      Operating System: All
      Platform: All

    • Issuezilla Id:
      108

      Description

      Calling BeanAdapter.release() multiple times causes a NullPointerException to be
      thrown. With a simple change outlined below the implementation of the
      release-method could be made more robust so that it doesn't throw an exception
      anymore.

      I know that the BeanAdpater instance must not be used anymore after calling
      release(). But on the other hand I've learned in the passed years that 'disposal
      code' should behave so robust, that disposing the same component multiple times
      should do no harm. For example, dialogs are often disposed multiple times: by
      user code and later by the runtime.
      Another example is when sharing the same PresentationModel with different dialog
      pages which are independent from each other. Every page gets released when the
      dialog is disposed. As a consequence, the shared PresentationModel will be
      released multiple times. Without a fix for this issue, user code must explicitly
      check this fact to avoid NPE's. This reduces usability and requires a major
      redesign of user code (for example in our framework).

      Here is the proposed change of method removeChangeHandlerFrom which is called by
      release():

      private void removeChangeHandlerFrom(Object bean)

      { // NEW: the null check of propertyChangeHandler if (!observeChanges || bean == null || propertyChangeHandler == null) return; BeanUtils.removePropertyChangeListener(bean, getBeanClass(bean), propertyChangeHandler); propertyChangeHandler = null; }

      That does not change any significant semantics and would increase usability.
      It would fit into version 1.2 because you already checked in changes for
      BeanAdaper.release() for this version.

      Thanks,
      Holger

        Activity

        Hide
        karsten added a comment -

        First off, this is not a bug but an enhancement for the documented behavior.
        I hesitate to start working on making corner cases more robust. There are other
        places with similar constraints.

        Anyway, I think I've outlined a solution for your situation that doesn't even
        require to call #release at all. If I remember it correctly, you can replace the
        bean channel before closing a dialog.

        Show
        karsten added a comment - First off, this is not a bug but an enhancement for the documented behavior. I hesitate to start working on making corner cases more robust. There are other places with similar constraints. Anyway, I think I've outlined a solution for your situation that doesn't even require to call #release at all. If I remember it correctly, you can replace the bean channel before closing a dialog.
        Hide
        karsten added a comment -

        I understand that this change may make sense for you. On the other hand, it
        increases the amount of constraints I've to manage, which is similar to a larger
        public API. I'm sorry to say that I can't justify the time to further increase
        the amount of constraints to manage in this project.

        However, I've just removed the final marker from the BeanAdapter, and the
        PresentationModel in version 1.3 allows to create a custom BeanAdapter. This
        makes it easier to change the BeanAdapter for developers.

        Anyway, I won't use #release at all, if it's unnecessary.

        Show
        karsten added a comment - I understand that this change may make sense for you. On the other hand, it increases the amount of constraints I've to manage, which is similar to a larger public API. I'm sorry to say that I can't justify the time to further increase the amount of constraints to manage in this project. However, I've just removed the final marker from the BeanAdapter, and the PresentationModel in version 1.3 allows to create a custom BeanAdapter. This makes it easier to change the BeanAdapter for developers. Anyway, I won't use #release at all, if it's unnecessary.
        Hide
        karsten added a comment -

        Since the project is quite stable now, there's more time to tweak corner cases.
        Hence, this feature may make it into the core.

        The NPE thrown when calling #release twice poorly indicates what's going on. I
        consider to do nothing if #release has been called before. Also, the docs must
        be extended for BeanAdapter, PresentationModel and all other classes that can
        #release.

        Show
        karsten added a comment - Since the project is quite stable now, there's more time to tweak corner cases. Hence, this feature may make it into the core. The NPE thrown when calling #release twice poorly indicates what's going on. I consider to do nothing if #release has been called before. Also, the docs must be extended for BeanAdapter, PresentationModel and all other classes that can #release.
        Hide
        karsten added a comment -

        #release can now be called multiple times in BeanAdapter, PresentationModel,
        PropertyAdapter, PropertyConnector, and TextComponentConnector.

        The change will be in version 2.0.4.

        Show
        karsten added a comment - #release can now be called multiple times in BeanAdapter, PresentationModel, PropertyAdapter, PropertyConnector, and TextComponentConnector. The change will be in version 2.0.4.

          People

          • Assignee:
            binding-issues
            Reporter:
            brands
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: