CSV-117 Validate format parameters in constructor

git-svn-id: https://svn.apache.org/repos/asf/commons/proper/csv/trunk@1603967 13f79535-47bb-0310-9956-ffa450edef68
This commit is contained in:
Sebastian Bazley 2014-06-19 17:28:22 +00:00
parent f4cc9a928f
commit 3cd50672f7
5 changed files with 32 additions and 32 deletions

View File

@ -40,6 +40,7 @@
<body> <body>
<release version="1.0" date="TBD" description="First release"> <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-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-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> <action issue="CSV-119" type="add" dev="ggregory" due-to="Sergei Lebedev">CSVFormat is missing a print(...) method</action>

View File

@ -327,6 +327,7 @@ public final class CSVFormat implements Serializable {
this.header = header.clone(); this.header = header.clone();
} }
this.skipHeaderRecord = skipHeaderRecord; this.skipHeaderRecord = skipHeaderRecord;
validate();
} }
@Override @Override
@ -666,38 +667,38 @@ public final class CSVFormat implements Serializable {
} }
/** /**
* 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()) { if (quoteChar != null && delimiter == quoteChar.charValue()) {
throw new IllegalStateException( throw new IllegalArgumentException(
"The quoteChar character and the delimiter cannot be the same ('" + quoteChar + "')"); "The quoteChar character and the delimiter cannot be the same ('" + quoteChar + "')");
} }
if (escape != null && delimiter == escape.charValue()) { if (escape != null && delimiter == escape.charValue()) {
throw new IllegalStateException( throw new IllegalArgumentException(
"The escape character and the delimiter cannot be the same ('" + escape + "')"); "The escape character and the delimiter cannot be the same ('" + escape + "')");
} }
if (commentStart != null && delimiter == commentStart.charValue()) { if (commentStart != null && delimiter == commentStart.charValue()) {
throw new IllegalStateException( throw new IllegalArgumentException(
"The comment start character and the delimiter cannot be the same ('" + commentStart + "')"); "The comment start character and the delimiter cannot be the same ('" + commentStart + "')");
} }
if (quoteChar != null && quoteChar.equals(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 + "')"); "The comment start character and the quoteChar cannot be the same ('" + commentStart + "')");
} }
if (escape != null && escape.equals(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 + "')"); "The comment start and the escape character cannot be the same ('" + commentStart + "')");
} }
if (escape == null && quotePolicy == Quote.NONE) { 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");
} }
} }

View File

@ -245,7 +245,6 @@ public final class CSVParser implements Iterable<CSVRecord>, Closeable {
Assertions.notNull(reader, "reader"); Assertions.notNull(reader, "reader");
Assertions.notNull(format, "format"); Assertions.notNull(format, "format");
format.validate();
this.format = format; this.format = format;
this.lexer = new Lexer(format, new ExtendedBufferedReader(reader)); this.lexer = new Lexer(format, new ExtendedBufferedReader(reader));
this.headerMap = this.initializeHeader(); this.headerMap = this.initializeHeader();

View File

@ -64,7 +64,6 @@ public final class CSVPrinter implements Flushable, Closeable {
this.out = out; this.out = out;
this.format = format; 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? // 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. // It seems a pain to have to track whether the header has already been printed or not.
if (format.getHeader() != null) { if (format.getHeader() != null) {

View File

@ -51,19 +51,19 @@ public class CSVFormatTest {
return format.withDelimiter(format.getDelimiter()); return format.withDelimiter(format.getDelimiter());
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testDelimiterSameAsCommentStartThrowsException() { public void testDelimiterSameAsCommentStartThrowsException() {
CSVFormat.DEFAULT.withDelimiter('!').withCommentStart('!').validate(); CSVFormat.DEFAULT.withDelimiter('!').withCommentStart('!');
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testDelimiterSameAsEscapeThrowsException() { public void testDelimiterSameAsEscapeThrowsException() {
CSVFormat.DEFAULT.withDelimiter('!').withEscape('!').validate(); CSVFormat.DEFAULT.withDelimiter('!').withEscape('!');
} }
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void testDuplicateHeaderElements() { public void testDuplicateHeaderElements() {
CSVFormat.DEFAULT.withHeader("A", "A").validate(); CSVFormat.DEFAULT.withHeader("A", "A");
} }
@Test @Test
@ -231,15 +231,15 @@ public class CSVFormatTest {
assertNotEquals(right, left); assertNotEquals(right, left);
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testEscapeSameAsCommentStartThrowsException() { public void testEscapeSameAsCommentStartThrowsException() {
CSVFormat.DEFAULT.withEscape('!').withCommentStart('!').validate(); CSVFormat.DEFAULT.withEscape('!').withCommentStart('!');
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testEscapeSameAsCommentStartThrowsExceptionForWrapperType() { public void testEscapeSameAsCommentStartThrowsExceptionForWrapperType() {
// Cannot assume that callers won't use different Character objects // 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 @Test
@ -272,25 +272,25 @@ public class CSVFormatTest {
assertFalse(formatStr.endsWith("null")); assertFalse(formatStr.endsWith("null"));
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testQuoteCharSameAsCommentStartThrowsException() { public void testQuoteCharSameAsCommentStartThrowsException() {
CSVFormat.DEFAULT.withQuoteChar('!').withCommentStart('!').validate(); CSVFormat.DEFAULT.withQuoteChar('!').withCommentStart('!');
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testQuoteCharSameAsCommentStartThrowsExceptionForWrapperType() { public void testQuoteCharSameAsCommentStartThrowsExceptionForWrapperType() {
// Cannot assume that callers won't use different Character objects // 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() { public void testQuoteCharSameAsDelimiterThrowsException() {
CSVFormat.DEFAULT.withQuoteChar('!').withDelimiter('!').validate(); CSVFormat.DEFAULT.withQuoteChar('!').withDelimiter('!');
} }
@Test(expected = IllegalStateException.class) @Test(expected = IllegalArgumentException.class)
public void testQuotePolicyNoneWithoutEscapeThrowsException() { public void testQuotePolicyNoneWithoutEscapeThrowsException() {
CSVFormat.newFormat('!').withQuotePolicy(Quote.NONE).validate(); CSVFormat.newFormat('!').withQuotePolicy(Quote.NONE);
} }
@Test @Test
@ -335,7 +335,7 @@ public class CSVFormatTest {
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void testWithCommentStartCRThrowsException() { public void testWithCommentStartCRThrowsException() {
CSVFormat.DEFAULT.withCommentStart(CR).validate(); CSVFormat.DEFAULT.withCommentStart(CR);
} }
@Test @Test
@ -346,7 +346,7 @@ public class CSVFormatTest {
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void testWithDelimiterLFThrowsException() { public void testWithDelimiterLFThrowsException() {
CSVFormat.DEFAULT.withDelimiter(LF).validate(); CSVFormat.DEFAULT.withDelimiter(LF);
} }
@Test @Test
@ -357,7 +357,7 @@ public class CSVFormatTest {
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void testWithEscapeCRThrowsExceptions() { public void testWithEscapeCRThrowsExceptions() {
CSVFormat.DEFAULT.withEscape(CR).validate(); CSVFormat.DEFAULT.withEscape(CR);
} }
@Test @Test
@ -399,7 +399,7 @@ public class CSVFormatTest {
@Test(expected = IllegalArgumentException.class) @Test(expected = IllegalArgumentException.class)
public void testWithQuoteLFThrowsException() { public void testWithQuoteLFThrowsException() {
CSVFormat.DEFAULT.withQuoteChar(LF).validate(); CSVFormat.DEFAULT.withQuoteChar(LF);
} }
@Test @Test