aokolnychyi commented on code in PR #5601:
URL: https://github.com/apache/iceberg/pull/5601#discussion_r953068758


##########
api/src/main/java/org/apache/iceberg/transforms/Bucket.java:
##########
@@ -159,111 +172,53 @@ public Type getResultType(Type sourceType) {
     return Types.IntegerType.get();
   }
 
-  private static class BucketInteger extends Bucket<Integer> {
+  private static class BucketInteger extends Bucket<Integer> implements 
Function<Integer, Integer> {
     private BucketInteger(int numBuckets) {
       super(numBuckets);
     }
 
     @Override
-    public int hash(Integer value) {
+    protected int hash(Integer value) {
       return BucketUtil.hash(value);
     }
-
-    @Override
-    public boolean canTransform(Type type) {
-      return type.typeId() == TypeID.INTEGER || type.typeId() == TypeID.DATE;
-    }
   }
 
-  private static class BucketLong extends Bucket<Long> {
+  private static class BucketLong extends Bucket<Long> implements 
Function<Long, Integer> {
     private BucketLong(int numBuckets) {
       super(numBuckets);
     }
 
     @Override
-    public int hash(Long value) {
+    protected int hash(Long value) {
       return BucketUtil.hash(value);
     }
-
-    @Override
-    public boolean canTransform(Type type) {
-      return type.typeId() == TypeID.LONG
-          || type.typeId() == TypeID.TIME
-          || type.typeId() == TypeID.TIMESTAMP;
-    }
   }
 
-  // bucketing by Double is not allowed by the spec, but this has the float 
hash implementation
-  static class BucketFloat extends Bucket<Float> {
-    // used by tests because the factory method will not instantiate a bucket 
function for floats
-    BucketFloat(int numBuckets) {
-      super(numBuckets);
-    }
-
-    @Override
-    public int hash(Float value) {
-      return BucketUtil.hash(value);
-    }
-
-    @Override
-    public boolean canTransform(Type type) {
-      return type.typeId() == TypeID.FLOAT;
-    }
-  }
-
-  // bucketing by Double is not allowed by the spec, but this has the double 
hash implementation
-  static class BucketDouble extends Bucket<Double> {
-    // used by tests because the factory method will not instantiate a bucket 
function for doubles
-    BucketDouble(int numBuckets) {
-      super(numBuckets);
-    }
-
-    @Override
-    public int hash(Double value) {
-      return BucketUtil.hash(value);
-    }
-
-    @Override
-    public boolean canTransform(Type type) {
-      return type.typeId() == TypeID.DOUBLE;
-    }
-  }
-
-  private static class BucketString extends Bucket<CharSequence> {
+  private static class BucketString extends Bucket<CharSequence>
+      implements Function<CharSequence, Integer> {

Review Comment:
   nit: What about adding an empty line if the class definition splits into 
multiple lines?
   There are a few classes like this.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to