HADOOP-16678: Review of ArrayWritable (#1692)

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/io/ArrayWritable.java

@@ -18,8 +18,10 @@
 
 package org.apache.hadoop.io;
 
-import java.io.*;
-import java.lang.reflect.Array;
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.util.Arrays;
 
 import org.apache.hadoop.classification.InterfaceAudience;
 import org.apache.hadoop.classification.InterfaceStability;
@@ -42,7 +44,7 @@
 @InterfaceAudience.Public
 @InterfaceStability.Stable
 public class ArrayWritable implements Writable {
-  private Class<? extends Writable> valueClass;
+  private final Class<? extends Writable> valueClass;
   private Writable[] values;
 
   public ArrayWritable(Class<? extends Writable> valueClass) {
@@ -64,7 +66,7 @@ public ArrayWritable(String[] strings) {
     }
   }
 
-  public Class getValueClass() {
+  public Class<? extends Writable> getValueClass() {
     return valueClass;
   }
 
@@ -77,16 +79,16 @@ public String[] toStrings() {
   }
 
   public Object toArray() {
-    Object result = Array.newInstance(valueClass, values.length);
-    for (int i = 0; i < values.length; i++) {
-      Array.set(result, i, values[i]);
-    }
-    return result;
+    return Arrays.copyOf(values, values.length);
   }
 
-  public void set(Writable[] values) { this.values = values; }
+  public void set(Writable[] values) {
+    this.values = values;
+  }
 
-  public Writable[] get() { return values; }
+  public Writable[] get() {
+    return values;
+  }
 
   @Override
   public void readFields(DataInput in) throws IOException {
@@ -106,5 +108,11 @@ public void write(DataOutput out) throws IOException {
     }
   }
 
+  @Override
+  public String toString() {
+    return "ArrayWritable [valueClass=" + valueClass + ", values="
+        + Arrays.toString(values) + "]";
+  }
+
 }
 

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/io/TestArrayWritable.java

@@ -18,12 +18,12 @@
 
 package org.apache.hadoop.io;
 
-import java.io.*;
-
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.junit.Assert.assertArrayEquals;
+
+import java.io.IOException;
+
 import org.junit.Test;
 
 
@@ -84,23 +84,14 @@ public void testArrayWritableToArray() {
   /**
    * test {@link ArrayWritable} constructor with null
    */
-  @Test
+  @Test(expected = IllegalArgumentException.class)
   public void testNullArgument() {
-    try {
-      Class<? extends Writable> valueClass = null;
-      new ArrayWritable(valueClass);
-      fail("testNullArgument error !!!");
-    } catch (IllegalArgumentException exp) {
-      //should be for test pass
-    } catch (Exception e) {
-      fail("testNullArgument error !!!");
-    }
+    new ArrayWritable((Class<? extends Writable>) null);
   }
 
   /**
    * test {@link ArrayWritable} constructor with {@code String[]} as a parameter
    */
-  @SuppressWarnings("deprecation")
   @Test
   public void testArrayWritableStringConstructor() {
     String[] original = { "test1", "test2", "test3" };