Refactor ProjectsCollector.collectProjects

[mthmulders]

This method now returns its result instead of modifying one of its arguments.

Following this checklist to help us incorporate your
contribution quickly and easily:

• Each commit in the pull request should have a meaningful subject line and body.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
• You have run the [Core IT][core-its] successfully.

If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement if you are unsure please ask on the developers list.

• I have signed the ICLA, and my employer has signed the CCLA.

[michael-o]

Can you tell what the purpose is behind this change?

[MartinKanters]

LGTM

[mthmulders]

Can you tell what the purpose is behind this change?

I identified this opportunity for improvement when I was working on MNG-6118, but that was complex enough already so I decided to postpone this little improvement and refactor it separately.

I think methods that return their results are better than methods that expect an argument which they can fill with a result, as the latter typically leads to code like this:

List<?> whatever = new ArrayList<>();
doSomething(whatever, other, arguments, that, are, needed);

which obscures the fact that the whatever will be modified by invoking doSomething. After this change, the same code would look like this:

List<?> whatever = doSomething(other, arguments, that, are, needed);

This is shorter and more clear (intention-revealing) IMO.

[michael-o]

There are two issues here:

1. You are changin a public interface. I don't know whether this is acceptable
2. As far as I know collect methods they are supposed to receive where to collect it, see the usage of ProblemCollector in Core.

I expect collectors to use used in a uniform fashion throughout the Core codebase.

[mthmulders]

1. This interface was merged into master only five days ago, I don't think anyone has built a different implementation for it just yet...
2. If I understood correctly, the ProblemCollector you are referring to is part of a chain. The collector we're talking about in this PR is not a part of chain. Would the same argument apply in that case, even at the expense (if I may say so) of having code that reveals its intent and is harder to interpret for the human brain?

[rfscholte]

I already saw the @todo in the previous PR, asking why a todo on new code. This is the finishing past. If the word collect is confusing, maybe select is better.

[michael-o]

@mthmulders I join @rfscholte opinion. The current naming isn't optimal.

[mthmulders]

Jenkins build is green.

[mthmulders]

Renamed both the class and the (public) method to be selector instead of collector. I assume everyone who reviewed agrees with this. If there's no objection today, I'll go ahead and merge into master.

[jira-importer]

Resolve #9734