keith-turner commented on code in PR #2811:
URL: https://github.com/apache/accumulo/pull/2811#discussion_r982437354
##########
server/tserver/src/main/java/org/apache/accumulo/tserver/scan/LookupTask.java:
##########
@@ -95,9 +95,14 @@ public void run() {
// check the time so that the read ahead thread is not monopolized
while (iter.hasNext() && bytesAdded < maxResultsSize
&& (System.currentTimeMillis() - startTime) < maxScanTime) {
- Entry<KeyExtent,List<Range>> entry = iter.next();
- KeyExtent extent = entry.getKey();
- List<Range> ranges = entry.getValue();
+ final KeyExtent extent;
+ final List<Range> ranges;
+ {
+ final Entry<KeyExtent,List<Range>> entry = iter.next();
+ extent = entry.getKey();
+ ranges = entry.getValue();
+ }
Review Comment:
> Yes. Maybe its not worth doing as it might cause confusion? What do you
think?
When I first saw it I had no idea why it was there. I looked at it for a
bit and guessed why it might be there. I think its an interesting technique and
it reminds me of how in golang a function can return zero or more things.
Where in java a functions can only return zero or one things. I really like
how golang works. To bad we can't write the following in java
```java
(entry,ranges) = iter.next();
```
Anyway, I am not completely sure if it should stay or go, but I am leaning
towards removing it. One thing that helped me think through this is asking
would this be a good practice throughout the code? It seems like this
technique could add a lot of noise that may make the code harder to read if
used frequently, so that's a reason not to use it elsewhere and here. The
tight scoping could help avoid some bugs in larger methods, but maybe if we
have a large method we just need to make it smaller like you have done for
methods in this PR.
--
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]
