[CSV-96] CSVRecord does not verify that the length of the header mapping matches the number of values - convert ArrayIndexOutOfBoundsException to IllegalArgumentException to give users a better feedback about what went wrong

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/csv/trunk@1465738 13f79535-47bb-0310-9956-ffa450edef68

Author: britter

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

@@ -55,7 +55,7 @@ public class CSVRecord implements Serializable, Iterable<String> {
 
     /**
      * Returns a value by index.
-     * 
+     *
      * @param i
      *            a column index (0-based)
      * @return the String at the given index
@@ -72,27 +72,37 @@ public String get(final int i) {
      * @return the column value, or {@code null} if the column name is not found
      * @throws IllegalStateException
      *             if no header mapping was provided
+     * @throws IllegalArgumentException
+     *             if the record is inconsistent
+     * @see #isConsistent()
      */
     public String get(final String name) {
         if (mapping == null) {
             throw new IllegalStateException(
                     "No header mapping was specified, the record values can't be accessed by name");
         }
         final Integer index = mapping.get(name);
-        return index != null ? values[index.intValue()] : null;
+        try {
+            return index != null ? values[index.intValue()] : null;
+        } catch (ArrayIndexOutOfBoundsException e) {
+            throw new IllegalArgumentException(
+                    String.format(
+                            "Index for header '%s' is %d but CSVRecord only has %d values!",
+                            name, index.intValue(), values.length));
+        }
     }
 
     /**
      * Returns true if this record is consistent, false if not. Currently, the only check is matching the record size to
      * the header size. Some programs can export files that fails this test but still produce parsable files.
-     * 
+     *
      * @return true of this record is valid, false if not
      * @see CSVParserTest#org.apache.commons.csv.CSVParserTest.testMappedButNotSetAsOutlook2007ContactExport()
      */
     public boolean isConsistent() {
         return mapping == null ? true : mapping.size() == values.length;
     }
-    
+
     /**
      * Checks whether a given column is mapped.
      *

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

@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.csv;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class CSVRecordTest {
+
+    private String[] values;
+    private CSVRecord record, recordWithHeader;
+    private Map<String, Integer> header;
+
+    @Before
+    public void setUp() throws Exception {
+        values = new String[] { "first", "second", "third" };
+        record = new CSVRecord(values, null, null, 0);
+        header = new HashMap<String, Integer>();
+        header.put("first", Integer.valueOf(0));
+        header.put("second", Integer.valueOf(1));
+        header.put("third", Integer.valueOf(2));
+        recordWithHeader = new CSVRecord(values, header, null, 0);
+    }
+
+    @Test
+    public void testGetInt() {
+        assertEquals(values[0], record.get(0));
+        assertEquals(values[1], record.get(1));
+        assertEquals(values[2], record.get(2));
+    }
+
+    @Test
+    public void testGetString() {
+        assertEquals(values[0], recordWithHeader.get("first"));
+        assertEquals(values[1], recordWithHeader.get("second"));
+        assertEquals(values[2], recordWithHeader.get("third"));
+    }
+
+    @Test(expected = IllegalStateException.class)
+    public void testGetStringNoHeader() {
+        record.get("first");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testGetStringInconsistentRecord() {
+        header.put("fourth", Integer.valueOf(4));
+        recordWithHeader.get("fourth");
+    }
+
+    @Test
+    public void testIsConsistent() {
+        assertTrue(record.isConsistent());
+        assertTrue(recordWithHeader.isConsistent());
+
+        header.put("fourth", Integer.valueOf(4));
+        assertFalse(recordWithHeader.isConsistent());
+    }
+
+    @Test
+    public void testIsMapped() {
+        assertFalse(record.isMapped("first"));
+        assertTrue(recordWithHeader.isMapped("first"));
+        assertFalse(recordWithHeader.isMapped("fourth"));
+    }
+
+    @Test
+    public void testIsSet() {
+        assertFalse(record.isSet("first"));
+        assertTrue(recordWithHeader.isSet("first"));
+        assertFalse(recordWithHeader.isSet("fourth"));
+    }
+
+    @Test
+    public void testIterator() {
+        int i = 0;
+        for (Iterator<String> itr = record.iterator(); itr.hasNext();) {
+            String value = itr.next();
+            assertEquals(values[i], value);
+            i++;
+        }
+    }
+
+}