[jira] [Created] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Created] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
Michael Pradel created COLLECTIONS-424:
------------------------------------------

             Summary: Surprising exception by CompositeSet in a situation where CompositeCollection works fine
                 Key: COLLECTIONS-424
                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
             Project: Commons Collections
          Issue Type: Bug
          Components: Set
    Affects Versions: 3.2.1
         Environment: All environments
            Reporter: Michael Pradel


We have a method that uses a CompositeCollection. Here's a simplified version of it:

  void m(CompositeCollection coll) {
    coll.addComposited(new TreeBag());
  }

It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:

  m(new CompositeCollection());  // OK
  m(new CompositeSet());         // IllegalArgumentException

Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.


A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:

  class CompositeCollection<T extends Collection> {
    void addComposited(T c) { /* .. */ }
  }
       
  class CompositeSet extends CompositeCollection<Set> {
    @Override void addComposited(Set c) { /* .. */ }
  }

This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)

    [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421525#comment-13421525 ]

srirangadeepthi commented on COLLECTIONS-424:
---------------------------------------------

I started working on this issue..but wondering how to assign this issue to myself..Can anybody help me on this..
               

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
In reply to this post by Gilles Sadowski (Jira)

    [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13421744#comment-13421744 ]

Thomas Neidhart commented on COLLECTIONS-424:
---------------------------------------------

Hi srirangadeepthi,

you can attach patches for this specific problem directly to the issue, there is no need to assign the issue itself to you.

Thomas
               

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       
Reply | Threaded
Open this post in threaded view
|

[jira] [Updated] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
In reply to this post by Gilles Sadowski (Jira)

     [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

srirangadeepthi updated COLLECTIONS-424:
----------------------------------------

    Attachment: collections424.patch
   

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>         Attachments: collections424.patch
>
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
In reply to this post by Gilles Sadowski (Jira)

    [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13446373#comment-13446373 ]

srirangadeepthi commented on COLLECTIONS-424:
---------------------------------------------

I have uploaded the patch. Here is the brief explanation of the changes i made in the current files.

1. Composite Collection holds the collections added to it in List<E> instead of List<Collection<E>>.This solves the issue of type checking in the code as mentioned in the bug.

2. I see add() and addComposited() performing the same functionality according to the current version.Whereas according to the apache commons collections doc,the functionalities are supposed to be different as shown below.

add(java.lang.Object obj)
          Adds an object to the collection, throwing UnsupportedOperationException unless a CollectionMutator strategy is specified.

addAll(java.util.Collection coll)
          Adds a collection of elements to this collection, throwing UnsupportedOperationException unless a CollectionMutator strategy is specified.

addComposited(java.util.Collection c)
          Add an additional collection to this composite.

But according to the current version,both does the same thing.In that case i don't see the need of having two methods for the same functionality.So i changed the functionality of the methods according to the document.

3. I changed the junit test cases accordingly and uploaded in the patch.
               

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>         Attachments: collections424.patch
>
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
In reply to this post by Gilles Sadowski (Jira)

    [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13451609#comment-13451609 ]

srirangadeepthi commented on COLLECTIONS-424:
---------------------------------------------

Hi..just wanted to remind about the patch uploaded.
               

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>         Attachments: collections424.patch
>
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (COLLECTIONS-424) Surprising exception by CompositeSet in a situation where CompositeCollection works fine

Gilles Sadowski (Jira)
In reply to this post by Gilles Sadowski (Jira)

    [ https://issues.apache.org/jira/browse/COLLECTIONS-424?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13459830#comment-13459830 ]

Thomas Neidhart commented on COLLECTIONS-424:
---------------------------------------------

Hi,

thanks for providing the patch, but unfortunately I am not sure if it goes into the right direction. Constraining a CompositeCollection to just one specific collection type is against the idea of the class (to provide a composite interface for a set of collections). Also the type safety for the element type is lost due to the fact that CompositeCollections extends Collection<Object>.

Tbh, I am more in favor of rejecting this issue or removing the inheritance to CompositeCollection in CompositeSet as there is no real need for it apart from code re-use.
               

> Surprising exception by CompositeSet in a situation where CompositeCollection works fine
> ----------------------------------------------------------------------------------------
>
>                 Key: COLLECTIONS-424
>                 URL: https://issues.apache.org/jira/browse/COLLECTIONS-424
>             Project: Commons Collections
>          Issue Type: Bug
>          Components: Set
>    Affects Versions: 3.2.1
>         Environment: All environments
>            Reporter: Michael Pradel
>         Attachments: collections424.patch
>
>
> We have a method that uses a CompositeCollection. Here's a simplified version of it:
>   void m(CompositeCollection coll) {
>     coll.addComposited(new TreeBag());
>   }
> It works fine when the argument is a CompositeCollection, but it throws an exception when the argument is a CompositeSet. E.g.:
>   m(new CompositeCollection());  // OK
>   m(new CompositeSet());         // IllegalArgumentException
> Although the exception is documented in CompositeSet, this behavior is very surprising. Is there a way to have m() accept CompositeCollections without running into this exception? The only solution that comes to my mind is to dynamically check the type of 'coll' in m(), but this is a rather nasty work-around.
> A better solution may be to make the genericity of CompositeCollection explicit by adding a type parameter:
>   class CompositeCollection<T extends Collection> {
>     void addComposited(T c) { /* .. */ }
>   }
>
>   class CompositeSet extends CompositeCollection<Set> {
>     @Override void addComposited(Set c) { /* .. */ }
>   }
> This way, users of CompositeCollection must choose the kind of collections that can be composed and will not encounter surprises, such as the above.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira