[GitHub] [hudi] xushiyan commented on a diff in pull request #7241: [HUDI-5241] Optimize HoodieDefaultTimeline API

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

xushiyan commented on code in PR #7241:
URL: https://github.com/apache/hudi/pull/7241#discussion_r1032912548


##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieTimeline.java:
##########
@@ -335,7 +337,18 @@ public interface HoodieTimeline extends Serializable {
   /**
    * @return Get the stream of completed instants
    */
-  Stream<HoodieInstant> getInstants();
+  default Stream<HoodieInstant> getInstantsAsStream() {
+    return getInstants().stream();
+  }
+
+  /**
+   * @return Get tht list of instants
+   */
+  List<HoodieInstant> getInstants();
+
+  default List<HoodieInstant> getAndCopyInstants() {
+    return new ArrayList<>(getInstants());
+  }

Review Comment:
   ditto
   
   also we should use `new ArrayList<>` for `getInstants()` and never expose `instants` to users. so we should just have `getInstants`, and make javadoc clear about the behavior and usage



##########
hudi-common/src/main/java/org/apache/hudi/common/table/timeline/HoodieTimeline.java:
##########
@@ -335,7 +337,18 @@ public interface HoodieTimeline extends Serializable {
   /**
    * @return Get the stream of completed instants
    */
-  Stream<HoodieInstant> getInstants();
+  default Stream<HoodieInstant> getInstantsAsStream() {
+    return getInstants().stream();
+  }

Review Comment:
   can we follow the existing convention to keep default impl. in HoodieDefaultTimeline? `getInstants()` is not implemented at this level hence `getInstantsAsStream()` may not be optimized to directly call `stream()`; for e.g. there could be a timeline subclass which is backed by stream of instants so you can return the stream directly instead of `getInstants().stream()`. let's leave the impl. to subclasses



-- 
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: commits-unsubscribe@hudi.apache.org

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