Commons BeanUtils
  1. Commons BeanUtils
  2. BEANUTILS-441

Replace UnmodifiableSet.decorate with Collections.unModifiableSet

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.9.0
    • Component/s: None
    • Labels:
      None

      Description

      Two methods in BeanUtils call UnmodifiableSet.decorate() from Commons Collections.

      These calls could be replaced with the standard Java method Collections.unmodifiableSet()

      1. BEANUTILS-441.patch
        0.9 kB
        samir kerroumi

        Activity

        Hide
        Oliver Heger added a comment -

        Fixed in subversion in revision 1540327. Thanks again for the patch.

        While adding generics to the API some other references to decorate() for obtaining unmodifiable collections have been replaced by their counterparts in the Collections class.

        Show
        Oliver Heger added a comment - Fixed in subversion in revision 1540327. Thanks again for the patch. While adding generics to the API some other references to decorate() for obtaining unmodifiable collections have been replaced by their counterparts in the Collections class.
        Hide
        samir kerroumi added a comment -

        Okay i take note of your advices

        Show
        samir kerroumi added a comment - Okay i take note of your advices
        Hide
        Sebb added a comment -

        Thanks, the patch dated 21/Jun/13 13:27 is much better.
        It only has two changes.

        It's always worth scanning the patch file for unexpected changes.

        Show
        Sebb added a comment - Thanks, the patch dated 21/Jun/13 13:27 is much better. It only has two changes. It's always worth scanning the patch file for unexpected changes.
        Hide
        samir kerroumi added a comment -

        thanks

        proud to contribute

        Show
        samir kerroumi added a comment - thanks proud to contribute
        Hide
        Benedikt Ritter added a comment -

        No need to be sorry, we always appreciate contributions from the community

        Show
        Benedikt Ritter added a comment - No need to be sorry, we always appreciate contributions from the community
        Hide
        samir kerroumi added a comment -

        I'm sorry for my first patch maybe i've some problems with my Ide considering code reformatfing

        i post a new version of this patch

        Show
        samir kerroumi added a comment - I'm sorry for my first patch maybe i've some problems with my Ide considering code reformatfing i post a new version of this patch
        Hide
        samir kerroumi added a comment -

        Yes i just saw this javadoc duplication

        but i cannot see the indentation differrence

        my modification is relatde to only two public methodes

        public Set keySet()

        and

        public Set entrySet()

        i repost the right patch

        apologize

        Show
        samir kerroumi added a comment - Yes i just saw this javadoc duplication but i cannot see the indentation differrence my modification is relatde to only two public methodes public Set keySet() and public Set entrySet() i repost the right patch apologize
        Hide
        Sebb added a comment -

        Thanks, but the patch changes some lines that are not related.
        Also the patch changes the indentation, so the difference is much larger than it need be.

        It also looks like the Javadoc for public Set keySet() has been duplicated with minor changes.

        It's much easier for patches to be reviewed (and maybe accepted) if they only change the minimum needed.

        Show
        Sebb added a comment - Thanks, but the patch changes some lines that are not related. Also the patch changes the indentation, so the difference is much larger than it need be. It also looks like the Javadoc for public Set keySet() has been duplicated with minor changes. It's much easier for patches to be reviewed (and maybe accepted) if they only change the minimum needed.
        Hide
        samir kerroumi added a comment - - edited

        Hi,

        This is my first patch.

        after modification i run BeanMaptestCase to ensure modification is correct

        Show
        samir kerroumi added a comment - - edited Hi, This is my first patch. after modification i run BeanMaptestCase to ensure modification is correct
        Hide
        Sebb added a comment -

        Other classes already call Collections.unmodifiableSet/Map/List

        Show
        Sebb added a comment - Other classes already call Collections.unmodifiableSet/Map/List
        Hide
        Sebb added a comment -

        BeanUtils also calls UnmodifiableList.decorate which could likewise be replaced.

        Show
        Sebb added a comment - BeanUtils also calls UnmodifiableList.decorate which could likewise be replaced.

          People

          • Assignee:
            Unassigned
            Reporter:
            Sebb
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development