CSVRecord.toList() does not give write access to the new List

Author: garydgregory

src/changes/changes.xml

@@ -46,6 +46,7 @@
       <action issue="CSV-269" type="fix" dev="ggregory" due-to="Auke te Winkel, Gary Gregory">CSVRecord.get(Enum) should use Enum.name() instead of Enum.toString().</action>
       <action                 type="fix" dev="ggregory" due-to="Gary Gregory">Allow org.apache.commons.csv.IOUtils.copy(Reader, Appendable, CharBuffer) to compile on Java 11 and run on Java 8.</action>
       <action                 type="fix" dev="ggregory" due-to="Gary Gregory">Bump commons-parent from 52 to 53.</action>
+      <action issue="CSV-300" type="fix" dev="ggregory" due-to="Markus Spann, Gary Gregory">CSVRecord.get(Enum) should use Enum.name() instead of Enum.toString().</action>
       <!-- ADD -->
       <action issue="CSV-291" type="add" dev="ggregory" due-to="Gary Gregory">Make CSVRecord#values() public.</action>
       <action issue="CSV-264" type="add" dev="ggregory" due-to="Sagar Tiwari, Seth Falco, Alex Herbert, Gary Gregory">Add DuplicateHeaderMode for flexibility with header strictness. #114.</action>

src/main/java/org/apache/commons/csv/CSVRecord.java

@@ -18,6 +18,7 @@
 package org.apache.commons.csv;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedHashMap;
@@ -298,17 +299,26 @@ public Stream<String> stream() {
     }
 
     /**
-     * Converts the values to a List.
+     * Converts the values to a new List.
+     * <p>
+     * Editing the list does not update this instance.
+     * </p>
      *
      * @return a new List
      * @since 1.9.0
      */
     public List<String> toList() {
-        return Arrays.asList(values);
+        // Only allocate a single list of known size
+        final ArrayList<String> list = new ArrayList<>(values.length);
+        Stream.of(values).forEach(list::add);
+        return list;
     }
 
     /**
      * Copies this record into a new Map of header name to record value.
+     * <p>
+     * Editing the map does not update this instance.
+     * </p>
      *
      * @return A new Map. The map is empty if the record has no headers.
      */

src/test/java/org/apache/commons/csv/CSVRecordTest.java

@@ -16,6 +16,7 @@
  */
 package org.apache.commons.csv;
 
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -30,6 +31,7 @@
 import java.io.ObjectOutputStream;
 import java.io.StringReader;
 import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
@@ -47,9 +49,7 @@ private enum EnumFixture {
 
     /** This enum overrides toString() but it's the names that matter. */
     public enum EnumHeader {
-        FIRST("first"),
-        SECOND("second"),
-        THIRD("third");
+        FIRST("first"), SECOND("second"), THIRD("third");
 
         private final String number;
 
@@ -69,7 +69,7 @@ public String toString() {
 
     @BeforeEach
     public void setUp() throws Exception {
-        values = new String[] {"A", "B", "C"};
+        values = new String[] { "A", "B", "C" };
         final String rowData = StringUtils.join(values, ',');
         try (final CSVParser parser = CSVFormat.DEFAULT.parse(new StringReader(rowData))) {
             record = parser.iterator().next();
@@ -273,14 +273,42 @@ public void testStream() {
     }
 
     @Test
-    public void testToList() {
+    public void testToListAdd() {
+        String[] expected = values.clone();
+        final List<String> list = record.toList();
+        list.add("Last");
+        assertEquals("Last", list.get(list.size() - 1));
+        assertEquals(list.size(), values.length + 1);
+        assertArrayEquals(expected, values);
+    }
+
+    @Test
+    public void testToListFor() {
         int i = 0;
         for (final String value : record.toList()) {
             assertEquals(values[i], value);
             i++;
         }
     }
 
+    @Test
+    public void testToListForEach() {
+        AtomicInteger i = new AtomicInteger();
+        record.toList().forEach(e -> {
+            assertEquals(values[i.getAndIncrement()], e);
+        });
+    }
+
+    @Test
+    public void testToListSet() {
+        String[] expected = values.clone();
+        final List<String> list = record.toList();
+        list.set(list.size() - 1, "Last");
+        assertEquals("Last", list.get(list.size() - 1));
+        assertEquals(list.size(), values.length);
+        assertArrayEquals(expected, values);
+    }
+
     @Test
     public void testToMap() {
         final Map<String, String> map = this.recordWithHeader.toMap();