Add a Collection option

[deluxe]

Add a Collection option to address #1879

[deluxe]

Hi there, first time contributor here! Please feel free ask any question.

[deluxe]

I named this method nextElement to overload nextElement(List<E>) but both methods could be named option like others in this class (with nextElement(List<E>) kept for backward compatibility).

[asolntsev]

Generally it's totally fine.

But in case of huge collections, this is not the most effective solution. List.copyOf(collection) will create a copy of the whole collection every time.

A more effective solution would be using Iterator, something like this:

int index = faker.random().nextInt(collection.size());
Iterator<T> it = collection.iterator();
for (int i = 0; i < index; i++) {
  iterator.next();
}
return iterator.next();

[kingthorin]

How about:

// Use streams, skip to random location in collection, return the first item there
return collection.stream()
            .skip(faker.random().nextInt(collection.size()))
            .findFirst();

[deluxe]

How about:

return collection.stream()
            .skip(faker.random().nextInt(collection.size()))
            .findFirst();

Would it randomize the output in case of an ordered collection ?

[kingthorin]

Yes, similar to the other approaches it would return a random n th element.

@asolntsev any feedback, stream vs iterator?

[deluxe]

Oh yes it's not a filter but a skip. My bad!

[asolntsev]

@kingthorin Sure, your solution with stream().skip() is more concise.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.61%. Comparing base (9da71f3) to head (6534166).
⚠️ Report is 12 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1880      +/-   ##
============================================
+ Coverage     92.48%   92.61%   +0.13%     
- Complexity     3562     3564       +2     
============================================
  Files           346      346              
  Lines          7048     7051       +3     
  Branches        685      678       -7     
============================================
+ Hits           6518     6530      +12     
+ Misses          366      362       -4     
+ Partials        164      159       -5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[asolntsev]

Approved - with one suggestion to improve performance.

[asolntsev]

Generally it's totally fine.

But in case of huge collections, this is not the most effective solution. List.copyOf(collection) will create a copy of the whole collection every time.

A more effective solution would be using Iterator, something like this:

int index = faker.random().nextInt(collection.size());
Iterator<T> it = collection.iterator();
for (int i = 0; i < index; i++) {
  iterator.next();
}
return iterator.next();

[kingthorin]

Thanks!

[asolntsev]

@deluxe Something is wrong with formatting in this PR.

Look at indentation here:

This is how usually formatting looks like in Java code:

[kingthorin]

That's on me, I did run spotless:apply but I may have still broken something after.
Will check/fix.

[deluxe]

You are too fast for me! If you prepare a v3, wouldn't it be the perfect time to harmonize the API by renaming Options.nextElement(...) to Options.option(...) ?

[kingthorin]

We could. I'd suggest opening a new issue in which to discuss. I don't think anyone has brought it up before.