ctubbsii commented on code in PR #3349:
URL: https://github.com/apache/accumulo/pull/3349#discussion_r1183213029
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java:
##########
@@ -149,8 +149,8 @@ private TabletMetadataLoader(Ample ample) {
loadAll(Set<? extends KeyExtent> keys) {
long t1 = System.currentTimeMillis();
@SuppressWarnings("unchecked")
- var tms = ample.readTablets().forTablets((Collection<KeyExtent>)
keys).build().stream()
- .collect(Collectors.toMap(tm -> tm.getExtent(), tm -> tm));
+ var tms = ample.readTablets().forTablets((Collection<KeyExtent>) keys, e
-> {}).build()
Review Comment:
I still think the named function is easier to understand, and doesn't
require the unnecessary wrapping/unwrapping of a Consumer into an Optional.
While not important for performance reasons, I just don't think the boilerplate
wrapping/unwrapping is necessary or very clean. There is plenty of precedent
for naming functions for reuse like this, either by assigning to a variable or
by using a method reference. I just think it's cleaner, and makes the code much
easier to read. But it's not that important to me (not enough to put in the
effort to persuade, anyway), and I guess the Optional does achieve a similar
purpose.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
