Add Stream counterparts to ReflectionSupport utility methods returning List

[kriegfrj]

Methods like ReflectionSupport.findField currently return a List<Field>.

Under-the-hood, the utils typically use streams and then copy all the elements into a list to return.

Very often, users of findfield() only need to iterate once through the returned list of fields (in some of my implementations, I've called stream() or forEach() on the result). Storing the stream of fields in an intermediate List is unnecessary buffering overhead in those cases.

Deliverables

• A set of methods with a signature like Stream<Field> findFieldsAsStream().
• Refactor the existing List<Field> findFields() to delegate to its equivalent streaming method and call toList() on the stream.
• Refactor existing usage in the JUnit 5 codebase where the stream interface would be more appropriate and efficient.

Thoughts?

[sbrannen]

Thoughts?

I understand the rationale and can see the desire for that, though I don't think it's a high priority to refactor the code base to go that route.

But maybe it's worth it to add such variants to ReflectionSupport for third parties.

Let's see what the rest of the team thinks.

[marcphilipp]

I see it similarly. I wouldn't object a contribution that adds such variants but also don't see it as a high priority for the team.

[marcphilipp]

Team decision: We'd be okay with a contribution that adds Stream variants of these methods but won't make it a priority.

[Attyuttam]

Hi, since this issue hasn't been assigned to anyone yet, can i please have a look into it ?

@marcphilipp /@sbrannen, so if I understand correctly, there are methods which are currently returning a list of Fields but we would want to return a Stream instead of the list to reduce the overhead ?

Will it be possible to direct me to the sections where these changes might be needed.

I am fairly new to contributing to open source so it would be great if you could pardon my questions if they seem too generic.

[marcphilipp]

@Attyuttam Sure, go for it! I would propose you start by implementing a Stream variant of ReflectionSupport.findFields called findFieldsAsStream and open a draft PR so we can see how involved that would be. Does that make sense to you?

[sbrannen]

Now that I've seen all of the proposed new names in #3084, I'm wondering if we should use stream* instead of *AsStream -- for example, streamAllClassesInClasspathRoot instead of findAllClassesInClasspathRootAsStream.

@junit-team/junit-lambda, thoughts?

[juliette-derancourt]

I agree that the *AsStream prefix does look a bit verbose, especially with long method names. This alternative seems more readable so I would be ok with it 👍

[kriegfrj]

👍

[Ndione24]

I agree with the nomenclature proposal. I just changed them and the pull request is available 👍

[sbrannen]

Hi @Ndione24,

I was actually suggesting that "stream" replace "find" in those methods.

So, streamAllClassesInClasspathRoot(...) instead of streamFindAllClassesInClasspathRoot(...), etc.

[Ndione24]

Hi @sbrannen,

I just renamed the methods :)