aditya1105 commented on code in PR #3919:
URL: https://github.com/apache/gobblin/pull/3919#discussion_r1569967666


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/watermark/StringWatermark.java:
##########
@@ -26,19 +26,22 @@
 
 import lombok.AllArgsConstructor;
 import lombok.EqualsAndHashCode;
-import lombok.Getter;
 
 
 /**
  * String based {@link ComparableWatermark} implementation.
  */
 @AllArgsConstructor
 @EqualsAndHashCode
-public class StringWatermark implements ComparableWatermark {
+public class StringWatermark implements ComparableWatermark<String> {
 
-  @Getter
   String value;
 
+  @Override
+  public String getValue(){

Review Comment:
   Why remove the getter here and add explicit method? For LongWatermark it was 
essential since return type needed to be object Long instead of primitive long.
   
   Also why is this value variable not private?



##########
gobblin-core-base/src/main/java/org/apache/gobblin/source/extractor/extract/LongWatermark.java:
##########
@@ -29,20 +29,23 @@
 import org.apache.gobblin.source.extractor.Watermark;
 
 import lombok.EqualsAndHashCode;
-import lombok.Getter;
 import lombok.Setter;
 import lombok.ToString;
 
 @ToString
 @EqualsAndHashCode
-public class LongWatermark implements ComparableWatermark {
+public class LongWatermark implements ComparableWatermark<Long> {
 
   private static final Gson GSON = new Gson();
 
-  @Getter
   @Setter
   private long value;
 
+  @Override
+  public Long getValue(){

Review Comment:
   We can probably add the explicit java doc for explicit use of long as class 
variable but Long as return value.
   
   ```
   Returns a Long object due to Java generics' requirement for object types.
   The underlying variable is maintained as a primitive long to optimize 
performance for mathematical operations.
   ```



-- 
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]

Reply via email to