Commons Collections
  1. Commons Collections
  2. COLLECTIONS-218

CollectionUtils.select() does not return passed in collection

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.2
    • Fix Version/s: 4.0-alpha1, 4.0
    • Component/s: Core
    • Labels:
      None

      Description

      collect has the following methods:

      Collection collect(Collection inputCollection, final Transformer transformer)
      Collection collect(Collection inputCollection, final Transformer transformer, final Collection outputCollection)

      The first creates an ArrayList(), where the second takes an outputCollection and, for convenience, returns it.

      Select (and selectReject) currently the following methods:

      Collection select(Collection inputCollection, Predicate predicate)
      void select(Collection inputCollection, Predicate predicate, Collection outputCollection)

      I propose changing the bottom method to return the passed in outputCollection. It would simplify calling code.

        Activity

        Hide
        Matt Benson added a comment -

        skestle resolved svn revision 593144

        Show
        Matt Benson added a comment - skestle resolved svn revision 593144
        Hide
        Stephen Colebourne added a comment -

        Implement for Generics

        Show
        Stephen Colebourne added a comment - Implement for Generics
        Hide
        Simon Kitching added a comment -

        I was a little surprised by the comment that changing void->non-void was binary incompatible, so I checked - and this is indeed the case. Thought I'd just add this comment to confirm it for anyone else wondering. It's a little odd, as method selection during compilation depends only on the parameter types, but my simple test shows it clearly:
        Exception in thread "main" java.lang.NoSuchMethodError: Target.process(Ljava/lang/String;)V
        that V at the end indicates the void return type. There is no longer a "Target.process" method taking String and returning void (because it was modified to return String) so..boom.

        Therefore I have to agree with Steven that unfortunately this change can't be applied.

        Show
        Simon Kitching added a comment - I was a little surprised by the comment that changing void->non-void was binary incompatible, so I checked - and this is indeed the case. Thought I'd just add this comment to confirm it for anyone else wondering. It's a little odd, as method selection during compilation depends only on the parameter types, but my simple test shows it clearly: Exception in thread "main" java.lang.NoSuchMethodError: Target.process(Ljava/lang/String;)V that V at the end indicates the void return type. There is no longer a "Target.process" method taking String and returning void (because it was modified to return String) so..boom. Therefore I have to agree with Steven that unfortunately this change can't be applied.
        Hide
        Stephen Colebourne added a comment -

        Its not intuitive, but its not just serialized classes.

        Our requirement is that other code compiled against collections will continue to work if the new jar is dropped in (without recompilation). Its just a 'feature' of Java that this can't be met.

        See http://www.eclipse.org/eclipse/development/java-api-evolution.html

        Show
        Stephen Colebourne added a comment - Its not intuitive, but its not just serialized classes. Our requirement is that other code compiled against collections will continue to work if the new jar is dropped in (without recompilation). Its just a 'feature' of Java that this can't be met. See http://www.eclipse.org/eclipse/development/java-api-evolution.html
        Hide
        Stephen Kestle added a comment -

        Binary compatibility only matters if the type is Serializable right? Is there something I'm missing? If someone is Serializing a static class, then it's a bit up the wall IMO.

        It's not going to break any client code, as they won't bu using a void return type.

        Is this just a business policy?

        Show
        Stephen Kestle added a comment - Binary compatibility only matters if the type is Serializable right? Is there something I'm missing? If someone is Serializing a static class, then it's a bit up the wall IMO. It's not going to break any client code, as they won't bu using a void return type. Is this just a business policy?
        Hide
        Stephen Colebourne added a comment -

        This change is logically sensible. However, it is backwards incompatible (changing a return type is binary incompatible). As such e cannot incorporate this proposal. Sorry.

        Show
        Stephen Colebourne added a comment - This change is logically sensible. However, it is backwards incompatible (changing a return type is binary incompatible). As such e cannot incorporate this proposal. Sorry.
        Hide
        Stephen Kestle added a comment -

        Uploaded patch file for proposed changes.

        I have also made changes to the two argument methods of select(), selectReject() and collect() to be simple one-liners:

        from:
        public static Collection collect(Collection inputCollection, Transformer transformer)

        { ArrayList answer = new ArrayList(inputCollection.size()); collect(inputCollection, transformer, answer); return answer; }

        to:
        public static Collection collect(Collection inputCollection, Transformer transformer)

        { return collect(inputCollection, transformer, new ArrayList(inputCollection.size())); }

        I'll leave it up to you developers to figure out if that's a good or a bad thing...

        Show
        Stephen Kestle added a comment - Uploaded patch file for proposed changes. I have also made changes to the two argument methods of select(), selectReject() and collect() to be simple one-liners: from: public static Collection collect(Collection inputCollection, Transformer transformer) { ArrayList answer = new ArrayList(inputCollection.size()); collect(inputCollection, transformer, answer); return answer; } to: public static Collection collect(Collection inputCollection, Transformer transformer) { return collect(inputCollection, transformer, new ArrayList(inputCollection.size())); } I'll leave it up to you developers to figure out if that's a good or a bad thing...

          People

          • Assignee:
            Stephen Kestle
            Reporter:
            Stephen Kestle
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development