You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by se...@apache.org on 2014/06/19 19:28:22 UTC

svn commit: r1603967 - in /commons/proper/csv/trunk/src: changes/ main/java/org/apache/commons/csv/ test/java/org/apache/commons/csv/

Author: sebb
Date: Thu Jun 19 17:28:22 2014
New Revision: 1603967

URL: http://svn.apache.org/r1603967
Log:
CSV-117 Validate format parameters in constructor

Modified:
    commons/proper/csv/trunk/src/changes/changes.xml
    commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
    commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
    commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java
    commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java

Modified: commons/proper/csv/trunk/src/changes/changes.xml
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/changes/changes.xml?rev=1603967&r1=1603966&r2=1603967&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/changes/changes.xml (original)
+++ commons/proper/csv/trunk/src/changes/changes.xml Thu Jun 19 17:28:22 2014
@@ -40,6 +40,7 @@
   <body>
 
     <release version="1.0" date="TBD" description="First release">    
+      <action issue="CSV-117" type="update" dev="sebb">Validate format parameters in constructor</action>
       <action issue="CSV-121" type="add" dev="ggregory" due-to="Sebastian Hardt">IllegalArgumentException thrown when the header contains duplicate names when the column names are empty.</action>
       <action issue="CSV-120" type="add" dev="ggregory" due-to="Sergei Lebedev">CSVFormat#withHeader doesn't work with CSVPrinter</action>
       <action issue="CSV-119" type="add" dev="ggregory" due-to="Sergei Lebedev">CSVFormat is missing a print(...) method</action>

Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java?rev=1603967&r1=1603966&r2=1603967&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java (original)
+++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java Thu Jun 19 17:28:22 2014
@@ -327,6 +327,7 @@ public final class CSVFormat implements 
             this.header = header.clone();
         }
         this.skipHeaderRecord = skipHeaderRecord;
+        validate();
     }
 
     @Override
@@ -666,38 +667,38 @@ public final class CSVFormat implements 
     }
 
     /**
-     * Verifies the consistency of the parameters and throws an IllegalStateException if necessary.
+     * Verifies the consistency of the parameters and throws an IllegalArgumentException if necessary.
      *
-     * @throws IllegalStateException
+     * @throws IllegalArgumentException
      */
-    void validate() throws IllegalStateException {
+    private void validate() throws IllegalArgumentException {
         if (quoteChar != null && delimiter == quoteChar.charValue()) {
-            throw new IllegalStateException(
+            throw new IllegalArgumentException(
                     "The quoteChar character and the delimiter cannot be the same ('" + quoteChar + "')");
         }
 
         if (escape != null && delimiter == escape.charValue()) {
-            throw new IllegalStateException(
+            throw new IllegalArgumentException(
                     "The escape character and the delimiter cannot be the same ('" + escape + "')");
         }
 
         if (commentStart != null && delimiter == commentStart.charValue()) {
-            throw new IllegalStateException(
+            throw new IllegalArgumentException(
                     "The comment start character and the delimiter cannot be the same ('" + commentStart + "')");
         }
 
         if (quoteChar != null && quoteChar.equals(commentStart)) {
-            throw new IllegalStateException(
+            throw new IllegalArgumentException(
                     "The comment start character and the quoteChar cannot be the same ('" + commentStart + "')");
         }
 
         if (escape != null && escape.equals(commentStart)) {
-            throw new IllegalStateException(
+            throw new IllegalArgumentException(
                     "The comment start and the escape character cannot be the same ('" + commentStart + "')");
         }
 
         if (escape == null && quotePolicy == Quote.NONE) {
-            throw new IllegalStateException("No quotes mode set but no escape character is set");
+            throw new IllegalArgumentException("No quotes mode set but no escape character is set");
         }
 
     }

Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java?rev=1603967&r1=1603966&r2=1603967&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java (original)
+++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java Thu Jun 19 17:28:22 2014
@@ -245,7 +245,6 @@ public final class CSVParser implements 
         Assertions.notNull(reader, "reader");
         Assertions.notNull(format, "format");
 
-        format.validate();
         this.format = format;
         this.lexer = new Lexer(format, new ExtendedBufferedReader(reader));
         this.headerMap = this.initializeHeader();

Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java?rev=1603967&r1=1603966&r2=1603967&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java (original)
+++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java Thu Jun 19 17:28:22 2014
@@ -64,7 +64,6 @@ public final class CSVPrinter implements
 
         this.out = out;
         this.format = format;
-        this.format.validate();
         // TODO: Is it a good idea to do this here instead of on the first call to a print method?
         // It seems a pain to have to track whether the header has already been printed or not.
         if (format.getHeader() != null) {

Modified: commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java?rev=1603967&r1=1603966&r2=1603967&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java (original)
+++ commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java Thu Jun 19 17:28:22 2014
@@ -51,19 +51,19 @@ public class CSVFormatTest {
         return format.withDelimiter(format.getDelimiter());
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testDelimiterSameAsCommentStartThrowsException() {
-        CSVFormat.DEFAULT.withDelimiter('!').withCommentStart('!').validate();
+        CSVFormat.DEFAULT.withDelimiter('!').withCommentStart('!');
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testDelimiterSameAsEscapeThrowsException() {
-        CSVFormat.DEFAULT.withDelimiter('!').withEscape('!').validate();
+        CSVFormat.DEFAULT.withDelimiter('!').withEscape('!');
     }
 
     @Test(expected = IllegalArgumentException.class)
     public void testDuplicateHeaderElements() {
-        CSVFormat.DEFAULT.withHeader("A", "A").validate();
+        CSVFormat.DEFAULT.withHeader("A", "A");
     }
 
     @Test
@@ -231,15 +231,15 @@ public class CSVFormatTest {
         assertNotEquals(right, left);
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testEscapeSameAsCommentStartThrowsException() {
-        CSVFormat.DEFAULT.withEscape('!').withCommentStart('!').validate();
+        CSVFormat.DEFAULT.withEscape('!').withCommentStart('!');
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testEscapeSameAsCommentStartThrowsExceptionForWrapperType() {
         // Cannot assume that callers won't use different Character objects
-        CSVFormat.DEFAULT.withEscape(new Character('!')).withCommentStart(new Character('!')).validate();
+        CSVFormat.DEFAULT.withEscape(new Character('!')).withCommentStart(new Character('!'));
     }
 
     @Test
@@ -272,25 +272,25 @@ public class CSVFormatTest {
         assertFalse(formatStr.endsWith("null"));
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testQuoteCharSameAsCommentStartThrowsException() {
-        CSVFormat.DEFAULT.withQuoteChar('!').withCommentStart('!').validate();
+        CSVFormat.DEFAULT.withQuoteChar('!').withCommentStart('!');
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testQuoteCharSameAsCommentStartThrowsExceptionForWrapperType() {
         // Cannot assume that callers won't use different Character objects
-        CSVFormat.DEFAULT.withQuoteChar(new Character('!')).withCommentStart('!').validate();
+        CSVFormat.DEFAULT.withQuoteChar(new Character('!')).withCommentStart('!');
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test//(expected = IllegalArgumentException.class)
     public void testQuoteCharSameAsDelimiterThrowsException() {
-        CSVFormat.DEFAULT.withQuoteChar('!').withDelimiter('!').validate();
+        CSVFormat.DEFAULT.withQuoteChar('!').withDelimiter('!');
     }
 
-    @Test(expected = IllegalStateException.class)
+    @Test(expected = IllegalArgumentException.class)
     public void testQuotePolicyNoneWithoutEscapeThrowsException() {
-        CSVFormat.newFormat('!').withQuotePolicy(Quote.NONE).validate();
+        CSVFormat.newFormat('!').withQuotePolicy(Quote.NONE);
     }
 
     @Test
@@ -335,7 +335,7 @@ public class CSVFormatTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void testWithCommentStartCRThrowsException() {
-        CSVFormat.DEFAULT.withCommentStart(CR).validate();
+        CSVFormat.DEFAULT.withCommentStart(CR);
     }
 
     @Test
@@ -346,7 +346,7 @@ public class CSVFormatTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void testWithDelimiterLFThrowsException() {
-        CSVFormat.DEFAULT.withDelimiter(LF).validate();
+        CSVFormat.DEFAULT.withDelimiter(LF);
     }
 
     @Test
@@ -357,7 +357,7 @@ public class CSVFormatTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void testWithEscapeCRThrowsExceptions() {
-        CSVFormat.DEFAULT.withEscape(CR).validate();
+        CSVFormat.DEFAULT.withEscape(CR);
     }
 
     @Test
@@ -399,7 +399,7 @@ public class CSVFormatTest {
 
     @Test(expected = IllegalArgumentException.class)
     public void testWithQuoteLFThrowsException() {
-        CSVFormat.DEFAULT.withQuoteChar(LF).validate();
+        CSVFormat.DEFAULT.withQuoteChar(LF);
     }
 
     @Test