[GitHub] [incubator-pegasus] foreverneverer commented on a diff in pull request #1019: refactor: Java client scan add hasNext method

[GitBox <gi...@apache.org>]

foreverneverer commented on code in PR #1019:
URL: https://github.com/apache/incubator-pegasus/pull/1019#discussion_r907106023


##########
java-client/src/main/java/org/apache/pegasus/client/PegasusScanner.java:
##########
@@ -88,9 +88,31 @@ public PegasusScanner(
     _needCheckHash = needCheckHash;
     _incomplete = false;
     _fullScan = fullScan;
+    _nextItem = null;
+  }
+
+  public boolean hasNext() throws PException {
+    synchronized (_nextItemLock) {
+      if (_incomplete) {
+        return false;
+      }
+      if (_rpcRunning && !_encounterError) {
+        return !_promises.isEmpty();
+      } else {
+        _nextItem = next();
+        return _nextItem != null;
+      }
+    }
   }
 
   public Pair<Pair<byte[], byte[]>, byte[]> next() throws PException {
+    synchronized (_nextItemLock) {

Review Comment:
   shouldn't use new `lock`, you should use `_promisesLock`



##########
java-client/src/main/java/org/apache/pegasus/client/PegasusScanner.java:
##########
@@ -88,9 +88,31 @@ public PegasusScanner(
     _needCheckHash = needCheckHash;
     _incomplete = false;
     _fullScan = fullScan;
+    _nextItem = null;
+  }
+
+  public boolean hasNext() throws PException {
+    synchronized (_nextItemLock) {
+      if (_incomplete) {
+        return false;
+      }
+      if (_rpcRunning && !_encounterError) {
+        return !_promises.isEmpty();
+      } else {
+        _nextItem = next();
+        return _nextItem != null;
+      }
+    }

Review Comment:
   I think you don't have to make such complex, just like follow seem to be ok:
   ```java
   public boolean hasNext() throws PException {
           _nextItem = next();
           return _nextItem != null;
   }
   ```
   
   `next()` method include enough logic to judge current status and make sure `thread-safe`. 
   
   That is to say `_incomplete`, `_rpcRunning && !_encounterError` should throw exception but return fasle



-- 
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: dev-unsubscribe@pegasus.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org