[Improvement] modify code to avoid raw use of parameterized class (#46)

### What changes were proposed in this pull request? 1. reduce the raw use of parameterized class 2. use `Collections.singletonList` instead of `Arrays.asList` for single element 3. add type annotation for some method signature and variable declaration ### Why are the changes needed? This makes javac and IntelliJ's compiler happy ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? Existing tests should be sufficient.
apache · Jul 10, 2022 · 1be8451 · 1be8451
1 parent ae674a6
commit 1be8451
Show file tree

Hide file tree

Showing 8 changed files with 18 additions and 17 deletions.
diff --git a/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java b/common/src/main/java/org/apache/uniffle/common/config/ConfigOptions.java
@@ -173,20 +173,20 @@ public static class TypedConfigOptionBuilder<T> {
       this.converter = converter;
     }
 
-    public ListConfigOptionBuilder asList() {
+    public ListConfigOptionBuilder<T> asList() {
       return new ListConfigOptionBuilder<T>(key, clazz, converter);
     }
 
     // todo: errorMsg shouldn't contain key
-    public TypedConfigOptionBuilder checkValue(Function<T, Boolean> checkValue, String errMsg) {
+    public TypedConfigOptionBuilder<T> checkValue(Function<T, Boolean> checkValue, String errMsg) {
       Function<Object, T> newConverter = (v) -> {
         T newValue = this.converter.apply(v);
         if (!checkValue.apply(newValue)) {
           throw new IllegalArgumentException(errMsg);
         }
         return newValue;
       };
-      return new TypedConfigOptionBuilder(key, clazz, newConverter);
+      return new TypedConfigOptionBuilder<>(key, clazz, newConverter);
     }
 
     /**
@@ -250,7 +250,7 @@ public ListConfigOptionBuilder(String key, Class<E> clazz, Function<Object, E> a
       };
     }
 
-    public ListConfigOptionBuilder checkValue(Function<E, Boolean> checkValueFunc, String errMsg) {
+    public ListConfigOptionBuilder<E> checkValue(Function<E, Boolean> checkValueFunc, String errMsg) {
       final Function<Object, List<E>> listConverFunc = asListConverter;
       Function<Object, List<E>> newConverter = (v) -> {
         List<E> list = listConverFunc.apply(v);

diff --git a/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java b/common/src/main/java/org/apache/uniffle/common/config/ConfigUtils.java
@@ -160,15 +160,15 @@ static Double convertToDouble(Object o) {
     return Double.parseDouble(o.toString());
   }
 
-  public static List<ConfigOption> getAllConfigOptions(Class confClass) {
-    List<ConfigOption> configOptionList = Lists.newArrayList();
+  public static List<ConfigOption<Object>> getAllConfigOptions(Class confClass) {
+    List<ConfigOption<Object>> configOptionList = Lists.newArrayList();
     try {
       Field[] fields = confClass.getFields();
       for (Field field : fields) {
         int modifiers = field.getModifiers();
         if (isStatic(modifiers) && isPublic(modifiers)
             && isFinal(modifiers) && field.getType().isAssignableFrom(ConfigOption.class)) {
-          configOptionList.add((ConfigOption) field.get(null));
+          configOptionList.add((ConfigOption<Object>) field.get(null));
         }
       }
     } catch (Exception e) {

diff --git a/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java b/common/src/main/java/org/apache/uniffle/common/config/RssBaseConf.java
@@ -162,7 +162,7 @@ public boolean loadCommonConf(Map<String, String> properties) {
       return false;
     }
 
-    List<ConfigOption> configOptions = ConfigUtils.getAllConfigOptions(RssBaseConf.class);
+    List<ConfigOption<Object>> configOptions = ConfigUtils.getAllConfigOptions(RssBaseConf.class);
     properties.forEach((k, v) -> {
       configOptions.forEach(config -> {
         if (config.key().equalsIgnoreCase(k)) {

diff --git a/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java b/common/src/test/java/org/apache/uniffle/common/util/RssUtilsTest.java
@@ -20,8 +20,8 @@
 import java.lang.reflect.Field;
 import java.net.InetAddress;
 import java.nio.ByteBuffer;
-import java.util.Arrays;
 import java.util.List;
+import java.util.Collections;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Random;
@@ -153,26 +153,26 @@ public void getMetricNameForHostNameTest() {
 
   @Test
   public void testLoadExtentions() {
-    List<String> exts = Arrays.asList("Dummy");
+    List<String> exts = Collections.singletonList("Dummy");
     try {
       RssUtils.loadExtensions(RssUtilTestDummy.class, exts, 1);
     } catch (RuntimeException e) {
       assertTrue(e.getMessage().startsWith("java.lang.ClassNotFoundException: Dummy"));
     }
-    exts = Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyFailNotSub");
+    exts = Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyFailNotSub");
     try {
       RssUtils.loadExtensions(RssUtilTestDummy.class, exts, 1);
     } catch (RuntimeException e) {
       assertTrue(e.getMessage().contains("RssUtilTestDummyFailNotSub is not subclass of "
           + "org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummy"));
     }
-    exts = Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyNoConstructor");
+    exts = Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummyNoConstructor");
     try {
       RssUtils.loadExtensions(RssUtilTestDummy.class, exts, "Test");
     } catch (RuntimeException e) {
       assertTrue(e.getMessage().contains("RssUtilTestDummyNoConstructor.<init>()"));
     }
-    exts = Arrays.asList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummySuccess");
+    exts = Collections.singletonList("org.apache.uniffle.common.util.RssUtilsTest$RssUtilTestDummySuccess");
     String testStr = String.valueOf(new Random().nextInt());
     List<RssUtilTestDummy> extsObjs = RssUtils.loadExtensions(RssUtilTestDummy.class, exts, testStr);
     assertEquals(1, extsObjs.size());
@@ -197,6 +197,7 @@ public String get() {
     }
   }
 
+  @SuppressWarnings("unchecked")
   public static void setEnv(String key, String value) {
     try {
       Map<String, String> env = System.getenv();

diff --git a/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java b/coordinator/src/main/java/org/apache/uniffle/coordinator/CoordinatorConf.java
@@ -140,7 +140,7 @@ public boolean loadConfFromFile(String fileName) {
 
     loadCommonConf(properties);
 
-    List<ConfigOption> configOptions = ConfigUtils.getAllConfigOptions(CoordinatorConf.class);
+    List<ConfigOption<Object>> configOptions = ConfigUtils.getAllConfigOptions(CoordinatorConf.class);
     properties.forEach((k, v) -> {
       configOptions.forEach(config -> {
         if (config.key().equalsIgnoreCase(k)) {

diff --git a/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java b/coordinator/src/test/java/org/apache/uniffle/coordinator/PartitionRangeAssignmentTest.java
@@ -33,7 +33,7 @@ public class PartitionRangeAssignmentTest {
 
   @Test
   public void test() {
-    SortedMap sortedMap = new TreeMap();
+    SortedMap<PartitionRange, List<ServerNode>> sortedMap = new TreeMap<>();
     for (int i = 0; i < 9; i = i + 3) {
       PartitionRange range = new PartitionRange(i, i + 2);
       List<ServerNode> nodes = Collections.singletonList(new ServerNode(

diff --git a/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java b/server/src/main/java/org/apache/uniffle/server/ShuffleServerConf.java
@@ -376,7 +376,7 @@ public boolean loadConfFromFile(String fileName) {
 
     loadCommonConf(properties);
 
-    List<ConfigOption> configOptions = ConfigUtils.getAllConfigOptions(ShuffleServerConf.class);
+    List<ConfigOption<Object>> configOptions = ConfigUtils.getAllConfigOptions(ShuffleServerConf.class);
 
     properties.forEach((k, v) -> {
       configOptions.forEach(config -> {

diff --git a/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java b/server/src/main/java/org/apache/uniffle/server/ShuffleUploader.java
@@ -413,7 +413,7 @@ List<ShuffleFileInfo> selectShuffleFiles(int num, boolean forceUpload) {
     return shuffleFileInfoList;
   }
 
-  private List getNotUploadedPartitions(String key) {
+  private List<Integer> getNotUploadedPartitions(String key) {
     RoaringBitmap bitmap = localStorage.getNotUploadedPartitions(key);
     List<Integer> partitionList = Lists.newArrayList();
     for (int p : bitmap) {