Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow methods to accept Iterable in addition to or in place of Collection #104

Open
tommack opened this issue Dec 29, 2012 · 4 comments
Open

Comments

@tommack
Copy link

tommack commented Dec 29, 2012

This would be a convenience for callers that only have an Iterable.

org.jdom2.ContentList.addAll(Collection<? extends Content>)

Is the most obvious candidate, but there may be others I haven't come across yet.

@rolfl
Copy link
Collaborator

rolfl commented Dec 30, 2012

Hi tommack.

Interesting idea. It is possibly one I should have considered before pushing out 2.x. Unfortunately it is more of a change than can be poushed through as a simple fix.... it will need to be part of 2.1.x not 2.0.x.

On the other hand, I have looked at the Content.addAll(Collection<...>) method, and it can be optimized a little bit. It does double-iteration through some collections (checks size() before iteration). There are some fixes available there.

I have a couple of items pending for 2.1.x and this can be added to the list.

Rolf

@tommack
Copy link
Author

tommack commented Jan 2, 2013

Sounds good. Thanks!

@rolfl
Copy link
Collaborator

rolfl commented Apr 29, 2013

It seems that people with code pre-compiled against JDOM 2.0.x will get NoSuchMethodError if running against 2.1.x when the method has had its signature changed from Collection<...> to Iterable<....>. Recompiling the 'client' code works fine, it picks up the new signature, and it works..... but....

... JDOM will have this thing happening often. What that means is that JDOM needs to keep both Collection and Iterable versions of the methods... I am trying to decide how much needs to change.... there's half a dozen places that make sense to change, and the duplication could be problematic...

Just a head's up that I am currently uncertain of the best solution.

@tommack
Copy link
Author

tommack commented Apr 30, 2013

In each location you could move the implementation to a new Iterable method and then delegate the Collection method to the Iterable method. But that doesn't really give you a path forward because clients will still compile against the Collection method unless they truly just have an Iterable. So, you still won't be able to delete the old method some day if you want binary compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants