ctubbsii commented on a change in pull request #1311:
URL: https://github.com/apache/zookeeper/pull/1311#discussion_r455501578
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/ZooKeeperServer.java
##########
@@ -488,17 +488,9 @@ public void loadData() throws IOException,
InterruptedException {
}
// Clean up dead sessions
- List<Long> deadSessions = new ArrayList<>();
- for (Long session : zkDb.getSessions()) {
- if (zkDb.getSessionWithTimeOuts().get(session) == null) {
- deadSessions.add(session);
- }
- }
-
- for (long session : deadSessions) {
- // TODO: Is lastProcessedZxid really the best thing to use?
- killSession(session, zkDb.getDataTreeLastProcessedZxid());
- }
+ zkDb.getSessions().stream()
Review comment:
@eolivelli Streams don't add much overhead, when used properly. They
might add some overhead sometimes, but that's not always the case, and even
when they do add a slight overhead, the increased reliability of the code, due
to its readability/maintainability can be worth the trade.
In this case, it:
1. Makes two loops (`O(n + m)`) into a single loop (`O(n)`),
2. Avoids creating an intermediate data structure entirely (saving 'm'
memory units), and
3. It's much nicer to read.
I see no reason to not apply these changes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
