On Mon, Nov 19, 2012 at 3:11 AM, Benedikt Ritter <benerit...@gmail.com>wrote:
> 2012/11/17 <ggreg...@apache.org> > > > Author: ggregory > > Date: Sat Nov 17 18:00:38 2012 > > New Revision: 1410759 > > > > URL: http://svn.apache.org/viewvc?rev=1410759&view=rev > > Log: > > [CSV-68] Use the Builder pattern for CSVFormat. > > > > Modified: > > > > > commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java > > > > > commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatBuilderTest.java > > > > > commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java > > > > 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=1410759&r1=1410758&r2=1410759&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 > > Sat Nov 17 18:00:38 2012 > > @@ -29,12 +29,13 @@ import java.io.IOException; > > import java.io.Reader; > > import java.io.Serializable; > > import java.io.StringWriter; > > +import java.util.Arrays; > > > > /** > > * The format specification of a CSV file. > > * > > * This class is immutable. > > - * > > + * > > * @version $Id$ > > */ > > public class CSVFormat implements Serializable { > > @@ -125,8 +126,8 @@ public class CSVFormat implements Serial > > > > /** > > * Creates a new CSV format builds. > > - * > > - * @param delimiter > > + * > > + * @param delimiter > > * the char used for value separation, must not be a line > > break character > > * @throws IllegalArgumentException if the delimiter is a line break > > character > > */ > > @@ -137,7 +138,7 @@ public class CSVFormat implements Serial > > public static CSVFormatBuilder newBuilder(final CSVFormat format) { > > return new CSVFormatBuilder(format); > > } > > - > > + > > /** > > * Standard comma separated format, as for {@link #RFC4180} but > > allowing blank lines. > > * <ul> > > @@ -158,7 +159,7 @@ public class CSVFormat implements Serial > > * the char used for value separation, must not be a line > > break character > > * @param quoteChar > > * the char used as value encapsulation marker > > - * @param quotePolicy > > + * @param quotePolicy > > * the quote policy > > * @param commentStart > > * the char used for comment identification > > @@ -174,10 +175,12 @@ public class CSVFormat implements Serial > > * the header > > * @throws IllegalArgumentException if the delimiter is a line break > > character > > */ > > - private CSVFormat(final char delimiter, final Character quoteChar, > > final Quote quotePolicy, final Character commentStart, final Character > > escape, final > > - boolean ignoreSurroundingSpaces, final boolean > > ignoreEmptyLines, final String lineSeparator, > > - final String[] header) { > > - if (isLineBreak(delimiter)) { > > + private CSVFormat(final char delimiter, final Character quoteChar, > > final Quote quotePolicy, final Character commentStart, final Character > > escape, final > > + boolean ignoreSurroundingSpaces, final boolean > > ignoreEmptyLines, final String lineSeparator, > > + final String[] header) > > + { > > + if (isLineBreak(delimiter)) > > + { > > throw new IllegalArgumentException("The delimiter cannot be > a > > line break"); > > } > > this.delimiter = delimiter; > > @@ -188,7 +191,7 @@ public class CSVFormat implements Serial > > this.ignoreSurroundingSpaces = ignoreSurroundingSpaces; > > this.ignoreEmptyLines = ignoreEmptyLines; > > this.recordSeparator = lineSeparator; > > - this.header = header; > > + this.header = header == null ? null : header.clone(); > > > > Well, that looks a lot simpler :) Thanks Gary! > What else has to be done to finish CSV-68? > Well, I think it is done as afar as getting something working but it would be best if you asked on the ML in a new thread. Then we can gather opinions as to what else it needs. Gary > > Regards, > Benedikt > > > > } > > > > /** > > @@ -373,7 +376,109 @@ public class CSVFormat implements Serial > > public Quote getQuotePolicy() { > > return quotePolicy; > > } > > - > > + > > + @Override > > + public int hashCode() > > + { > > + final int prime = 31; > > + int result = 1; > > + > > + result = prime * result + delimiter; > > + result = prime * result + ((quotePolicy == null) ? 0 : > > quotePolicy.hashCode()); > > + result = prime * result + ((quoteChar == null) ? 0 : > > quoteChar.hashCode()); > > + result = prime * result + ((commentStart == null) ? 0 : > > commentStart.hashCode()); > > + result = prime * result + ((escape == null) ? 0 : > > escape.hashCode()); > > + result = prime * result + (ignoreSurroundingSpaces ? 1231 : > 1237); > > + result = prime * result + (ignoreEmptyLines ? 1231 : 1237); > > + result = prime * result + ((recordSeparator == null) ? 0 : > > recordSeparator.hashCode()); > > + result = prime * result + Arrays.hashCode(header); > > + return result; > > + } > > + > > + @Override > > + public boolean equals(Object obj) > > + { > > + if (this == obj) > > + { > > + return true; > > + } > > + if (obj == null) > > + { > > + return false; > > + } > > + if (getClass() != obj.getClass()) > > + { > > + return false; > > + } > > + > > + CSVFormat other = (CSVFormat) obj; > > + if (delimiter != other.delimiter) > > + { > > + return false; > > + } > > + if (quotePolicy != other.quotePolicy) > > + { > > + return false; > > + } > > + if (quoteChar == null) > > + { > > + if (other.quoteChar != null) > > + { > > + return false; > > + } > > + } > > + else if (!quoteChar.equals(other.quoteChar)) > > + { > > + return false; > > + } > > + if (commentStart == null) > > + { > > + if (other.commentStart != null) > > + { > > + return false; > > + } > > + } > > + else if (!commentStart.equals(other.commentStart)) > > + { > > + return false; > > + } > > + if (escape == null) > > + { > > + if (other.escape != null) > > + { > > + return false; > > + } > > + } > > + else if (!escape.equals(other.escape)) > > + { > > + return false; > > + } > > + if (!Arrays.equals(header, other.header)) > > + { > > + return false; > > + } > > + if (ignoreSurroundingSpaces != other.ignoreSurroundingSpaces) > > + { > > + return false; > > + } > > + if (ignoreEmptyLines != other.ignoreEmptyLines) > > + { > > + return false; > > + } > > + if (recordSeparator == null) > > + { > > + if (other.recordSeparator != null) > > + { > > + return false; > > + } > > + } > > + else if (!recordSeparator.equals(other.recordSeparator)) > > + { > > + return false; > > + } > > + return true; > > + } > > + > > public static class CSVFormatBuilder { > > > > private char delimiter; > > @@ -393,7 +498,7 @@ public class CSVFormat implements Serial > > * the char used for value separation, must not be a > > line break character > > * @param quoteChar > > * the char used as value encapsulation marker > > - * @param quotePolicy > > + * @param quotePolicy > > * the quote policy > > * @param commentStart > > * the char used for comment identification > > @@ -410,8 +515,8 @@ public class CSVFormat implements Serial > > * @throws IllegalArgumentException if the delimiter is a line > > break character > > */ > > // package protected for use by test code > > - CSVFormatBuilder(final char delimiter, final Character > quoteChar, > > final Quote quotePolicy, final Character commentStart, final Character > > escape, final > > - boolean ignoreSurroundingSpaces, final boolean > > ignoreEmptyLines, final String lineSeparator, > > + CSVFormatBuilder(final char delimiter, final Character > quoteChar, > > final Quote quotePolicy, final Character commentStart, final Character > > escape, final > > + boolean ignoreSurroundingSpaces, final boolean > > ignoreEmptyLines, final String lineSeparator, > > final String[] header) { > > if (isLineBreak(delimiter)) { > > throw new IllegalArgumentException("The delimiter cannot > > be a line break"); > > @@ -426,11 +531,11 @@ public class CSVFormat implements Serial > > this.recordSeparator = lineSeparator; > > this.header = header; > > } > > - > > + > > /** > > - * > > + * > > * Creates a CSVFormatBuilder, using the values of the given > > CSVFormat. > > - * > > + * > > * @param format > > * The format to use values from > > */ > > @@ -443,8 +548,8 @@ public class CSVFormat implements Serial > > > > /** > > * Creates a basic CSVFormatBuilder. > > - * > > - * @param delimiter > > + * > > + * @param delimiter > > * the char used for value separation, must not be a > > line break character > > * @throws IllegalArgumentException if the delimiter is a line > > break character > > */ > > @@ -457,35 +562,35 @@ public class CSVFormat implements Serial > > return new CSVFormat(delimiter, quoteChar, quotePolicy, > > commentStart, escape, > > ignoreSurroundingSpaces, > > ignoreEmptyLines, recordSeparator, header); > > } > > - > > + > > /** > > * Verifies the consistency of the parameters and throws an > > IllegalStateException if necessary. > > - * > > + * > > * @throws IllegalStateException > > */ > > private void validate() throws IllegalStateException { > > if (quoteChar != null && delimiter == > quoteChar.charValue()) { > > throw new IllegalStateException("The quoteChar character > > and the delimiter cannot be the same ('" + quoteChar + "')"); > > } > > - > > + > > if (escape != null && delimiter == escape.charValue()) { > > throw new IllegalStateException("The escape character > and > > the delimiter cannot be the same ('" + escape + "')"); > > } > > - > > + > > if (commentStart != null && delimiter == > > commentStart.charValue()) { > > - throw new IllegalStateException("The comment start > > character and the delimiter cannot be the same ('" + commentStart + > > + throw new IllegalStateException("The comment start > > character and the delimiter cannot be the same ('" + commentStart + > > "')"); > > } > > - > > + > > if (quoteChar != null && quoteChar.equals(commentStart)) { > > - throw new IllegalStateException("The comment start > > character and the quoteChar cannot be the same ('" + commentStart + > > + throw new IllegalStateException("The comment start > > character and the quoteChar cannot be the same ('" + commentStart + > > "')"); > > } > > - > > + > > if (escape != null && escape.equals(commentStart)) { > > throw new IllegalStateException("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"); > > } > > @@ -504,7 +609,7 @@ public class CSVFormat implements Serial > > if (isLineBreak(delimiter)) { > > throw new IllegalArgumentException("The delimiter cannot > > be a line break"); > > } > > - this.delimiter = delimiter; > > + this.delimiter = delimiter; > > return this; > > } > > > > @@ -625,7 +730,7 @@ public class CSVFormat implements Serial > > this.header = header; > > return this; > > } > > - > > + > > /** > > * Sets the trimming behavior of the format. > > * > > > > Modified: > > > commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatBuilderTest.java > > URL: > > > http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatBuilderTest.java?rev=1410759&r1=1410758&r2=1410759&view=diff > > > > > ============================================================================== > > --- > > > commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatBuilderTest.java > > (original) > > +++ > > > commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatBuilderTest.java > > Sat Nov 17 18:00:38 2012 > > @@ -21,9 +21,9 @@ import static org.apache.commons.csv.CSV > > import static org.apache.commons.csv.Constants.CR; > > import static org.apache.commons.csv.Constants.CRLF; > > import static org.apache.commons.csv.Constants.LF; > > -import static org.junit.Assert.assertArrayEquals; > > import static org.junit.Assert.assertEquals; > > import static org.junit.Assert.assertFalse; > > +import static org.junit.Assert.assertNotSame; > > import static org.junit.Assert.assertTrue; > > > > import org.apache.commons.csv.CSVFormat.CSVFormatBuilder; > > @@ -159,7 +159,7 @@ public class CSVFormatBuilderTest { > > @Test > > public void testCopiedFormatIsEqualToOriginal() { > > CSVFormat copyOfRCF4180 = CSVFormat.newBuilder(RFC4180).build(); > > - assertEqualFormats(RFC4180, copyOfRCF4180); > > + assertEquals(RFC4180, copyOfRCF4180); > > } > > > > @Test > > @@ -168,16 +168,14 @@ public class CSVFormatBuilderTest { > > assertTrue(newFormat.getDelimiter() != RFC4180.getDelimiter()); > > } > > > > - // FIXME implement equals on CSVFormat to allow use of > > Assert.assertEquals() > > - private static void assertEqualFormats(CSVFormat expected, CSVFormat > > acutal) { > > - assertEquals(expected.getCommentStart(), > > acutal.getCommentStart()); > > - assertEquals(expected.getDelimiter(), acutal.getDelimiter()); > > - assertEquals(expected.getEscape(), acutal.getEscape()); > > - assertArrayEquals(expected.getHeader(), acutal.getHeader()); > > - assertEquals(expected.getIgnoreEmptyLines(), > > acutal.getIgnoreEmptyLines()); > > - assertEquals(expected.getIgnoreSurroundingSpaces(), > > acutal.getIgnoreSurroundingSpaces()); > > - assertEquals(expected.getQuoteChar(), acutal.getQuoteChar()); > > - assertEquals(expected.getQuotePolicy(), > acutal.getQuotePolicy()); > > - assertEquals(expected.getRecordSeparator(), > > acutal.getRecordSeparator()); > > + @Test > > + public void testHeaderReferenceCannotEscape() { > > + String[] header = new String[]{"one", "tow", "three"}; > > + builder.withHeader(header); > > + > > + CSVFormat firstFormat = builder.build(); > > + CSVFormat secondFormat = builder.build(); > > + assertNotSame(header, firstFormat.getHeader()); > > + assertNotSame(firstFormat, secondFormat.getHeader()); > > } > > } > > > > 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=1410759&r1=1410758&r2=1410759&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 > > Sat Nov 17 18:00:38 2012 > > @@ -18,6 +18,7 @@ > > package org.apache.commons.csv; > > > > import static org.junit.Assert.assertEquals; > > +import static org.junit.Assert.assertFalse; > > import static org.junit.Assert.assertNotNull; > > > > import java.io.ByteArrayInputStream; > > @@ -65,4 +66,153 @@ public class CSVFormatTest { > > assertEquals("trim", > > CSVFormat.DEFAULT.getIgnoreSurroundingSpaces(), > > format.getIgnoreSurroundingSpaces()); > > assertEquals("empty lines", > > CSVFormat.DEFAULT.getIgnoreEmptyLines(), format.getIgnoreEmptyLines()); > > } > > + > > + @Test > > + public void testEquals() { > > + CSVFormat right = CSVFormat.DEFAULT; > > + CSVFormat left = CSVFormat.newBuilder().build(); > > + > > + assertFalse(right.equals(null)); > > + assertFalse(right.equals("A String Instance")); > > + > > + assertEquals(right, right); > > + assertEquals(right, left); > > + assertEquals(left, right); > > + > > + assertEquals(right.hashCode(), right.hashCode()); > > + assertEquals(right.hashCode(), left.hashCode()); > > + } > > + > > + @Test > > + public void testEqualsDelimiter() { > > + CSVFormat right = CSVFormat.newBuilder('!').build(); > > + CSVFormat left = CSVFormat.newBuilder('?').build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsQuoteChar() { > > + CSVFormat right = > > CSVFormat.newBuilder('\'').withQuoteChar('"').build(); > > + CSVFormat left = > > CSVFormat.newBuilder(right).withQuoteChar('!').build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsQuotePolicy() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withQuotePolicy(Quote.MINIMAL) > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsCommentStart() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withCommentStart('!') > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsEscape() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .withEscape('+') > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withEscape('!') > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsIgnoreSurroundingSpaces() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .withEscape('+') > > + .withIgnoreSurroundingSpaces(true) > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withIgnoreSurroundingSpaces(false) > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsIgnoreEmptyLines() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .withEscape('+') > > + .withIgnoreSurroundingSpaces(true) > > + .withIgnoreEmptyLines(true) > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withIgnoreEmptyLines(false) > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsRecordSeparator() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .withEscape('+') > > + .withIgnoreSurroundingSpaces(true) > > + .withIgnoreEmptyLines(true) > > + .withRecordSeparator('*') > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withRecordSeparator('!') > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + @Test > > + public void testEqualsHeader() { > > + CSVFormat right = CSVFormat.newBuilder('\'') > > + .withQuoteChar('"') > > + .withQuotePolicy(Quote.ALL) > > + .withCommentStart('#') > > + .withEscape('+') > > + .withIgnoreSurroundingSpaces(true) > > + .withIgnoreEmptyLines(true) > > + .withRecordSeparator('*') > > + .withHeader("One", "Two", "Three") > > + .build(); > > + CSVFormat left = CSVFormat.newBuilder(right) > > + .withHeader("Three", "Two", "One") > > + .build(); > > + > > + assertNotEquals(right, left); > > + } > > + > > + private static void assertNotEquals(Object right, Object left) { > > + assertFalse(right.equals(left)); > > + assertFalse(left.equals(right)); > > + } > > } > > > > > > > -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org JUnit in Action, 2nd Ed: <http://goog_1249600977>http://bit.ly/ECvg0 Spring Batch in Action: <http://s.apache.org/HOq>http://bit.ly/bqpbCK Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
