-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[LANG-927] Add method convert a iterator to array in ArrayUtils #1411
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
Conversation
Add method convert a iterator to array in ArrayUtils.
Hi @garydgregory, could you please review this PR when you have time? I’d really appreciate your feedback. |
This looks like a duplicate for https://commons.apache.org/proper/commons-collections/apidocs/org/apache/commons/collections4/IteratorUtils.html |
@garydgregory Thanks for pointing out, I think we should close this PR and jira-issue. WDYT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Abyss-lord,
Thank you for your PR!
At first glance, I'm not convinced that the proposed methods offer a clear improvement over existing options. For example, consider the following comparisons:
Iterator<String> iterator = ...
Object[] objectArray = ArrayUtils.iteratorToArray(iterator);
String[] stringArray = ArrayUtils.iteratorToArray(iterator, String.class);
versus the equivalent using existing Streams
support:
Iterator<String> iterator = ...
Object[] objectArray = Streams.of(iterator).toArray();
String[] stringArray = Streams.of(iterator).toArray(String[]::new);
Functionally, these are very similar. The main difference is that the ArrayUtils
versions return null
when the input is null
, which—depending on perspective—can be a pro or a con. Personally, I view this as a downside: I prefer to eliminate null
s early and treat empty arrays and null
s as equivalent.
Could you share more context on your use case or motivation for introducing these methods? That might help clarify their intended value.
Also, setting aside the question of inclusion, I believe the proposed implementations need some adjustments. I've left comments on the relevant lines for review.
Thanks again for the contribution—looking forward to your thoughts!
Right, nice catch! 😉 |
@ppkarwasz , thank you for the detailed review and thoughtful feedback! The reason I added this method is because I came across a related request in a JIRA issue, where someone expressed the need for this functionality. As a newcomer to the Commons Lang community, I wanted to start contributing by picking up something small but potentially useful. Also, could you please advise where I can find guidelines or existing examples to help identify the kinds of utility methods that are considered suitable for inclusion in Commons Lang? That would really help me better align future contributions. Thanks again! |
That’s a great question — and one that @garydgregory is best positioned to answer. That said, here are my two cents based on experience: What Commons Lang Is ForCommons Lang was originally created to fill gaps in early JDKs — to provide utility methods that were missing or cumbersome to implement. Classic examples include:
How the JDK Has EvolvedMany of the original pain points Commons Lang addressed have since been resolved in newer JDK versions:
In SummaryIt’s getting harder to find genuinely useful additions to Commons Lang — and that's a good thing. The JDK has improved a lot. Rather than expanding Commons Lang with new utilities, it might be more helpful to offer guidance or utilities to help users migrate away from it in favor of modern JDK APIs where possible. But again, for definitive inclusion guidelines or strategic direction, I’d defer to @garydgregory or |
While I mostly agree with @ppkarwasz, I disagree with migrating away from the library. WRT using APIs in the fluent style, there has been a recent discussion (either in Jira, GitHub, or the mailing list, sorry I can't recall) about normalizing null versus I do not have specific guidance to give @Abyss-lord on what to add out of thin air, as I do not have a to-do list for new methods and Jira doesn't have tickets for new code to add, IIRC. What Jira does have, on the other hand, are bug reports to process, research, and propose changes. In general, I would say that Lang additions reflect real-world use cases from other libraries and applications that I've dealt with in the past in both an open-source and corporate context. HTH |
Closing: This PR duplicates code in Commons Collection. |
This PR fixes issue LANG-927 which ask to "Create Iterable APIs"
✅ Improvement:
Create Iterable APIs, add two methods in
ArrayUtils
.T[] iteratorToArray(Iterator<T> iterator)
T[] iteratorToArray(Iterator<T> iterator, Class<T> clazz)
✅ Tests:
Added two test methods in
ArrayUtilsTest
and All other tests passtestIteratorToArrayWithClazz
testIteratorToArray