I'd prefer
public void checkRetrievedIds( List<Integer> expectedIds, List<Integer> retrievedIds ) {
Set<Integer> difference = Sets.difference(
new HashSet<>( expectedIds ),
new HashSet<>( retrievedIds )
);
createInvalidationEvents( difference );
}
You're right that your first version is somewhat verbose for the task. However, there are some good reasons to separate out the difference calculation from the function call. First, what if you wanted to do something else with the difference? You'd immediately have to refactor to something like this version so as to have the difference to manipulate.
I'm also somewhat leery of a function that consists entirely of chained calls. One has to sort of mentally unwrap it in order to get to the real meaning. They often make a lot of sense when writing them, as you know exactly what you are trying to accomplish. They can make a lot less sense six months down the road.
Separating the two allows us to basically have one line that calculates the set difference of two lists while the other line makes use of that difference. That seems a reasonable division of responsibility.
Another possibility would be to change the signature of createInvalidationEvents
to take two lists as input. Then you wouldn't need checkRetrievedIds
. Of course, that wouldn't work if createInvalidationEvents
is used elsewhere with the current signature.
A third possibility would be to calculate the set difference in whatever calls checkRetrievedIds
. This would also get rid of checkRetrievedIds
.
As written, checkRetrievedIds
is simply an alias for createInvalidationEvents
with a different method signature. Without other context, it's unclear why you need it.