You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by sebb <se...@gmail.com> on 2012/10/12 15:44:06 UTC

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

On 12 October 2012 14:15,  <gg...@apache.org> wrote:
> Author: ggregory
> Date: Fri Oct 12 13:15:30 2012
> New Revision: 1397556
>
> URL: http://svn.apache.org/viewvc?rev=1397556&view=rev
> Log:
> More constants clean ups.
>
> Modified:
>     commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
>     commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
>     commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java
>     commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.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=1397556&r1=1397555&r2=1397556&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 Fri Oct 12 13:15:30 2012
> @@ -18,7 +18,7 @@
>  package org.apache.commons.csv;
>
>  import static org.apache.commons.csv.Constants.COMMA;
> -import static org.apache.commons.csv.Constants.CR;
> +import static org.apache.commons.csv.Constants.CRLF;
>  import static org.apache.commons.csv.Constants.DOUBLE_QUOTE;
>  import static org.apache.commons.csv.Constants.ESCAPE;
>  import static org.apache.commons.csv.Constants.LF;
> @@ -36,13 +36,8 @@ import java.io.StringWriter;
>   */
>  public class CSVFormat implements Serializable {
>
> -    private static final String LF_STR = "" + LF;
> -
>      private static final long serialVersionUID = 1L;
>
> -    /** According to RFC 4180, line breaks are delimited by CRLF */
> -    public static final String CRLF = "" + CR + LF;
> -
>      private final char delimiter;
>      private final char encapsulator;
>      private final char commentStart;
> @@ -136,7 +131,7 @@ public class CSVFormat implements Serial
>              PRISTINE
>              .withDelimiter(TAB)
>              .withEscape(ESCAPE)
> -            .withLineSeparator(LF_STR);
> +            .withLineSeparator(LF);
>
>      /**
>       * Creates a customized CSV format.
> @@ -413,6 +408,19 @@ public class CSVFormat implements Serial
>       *
>       * @return A copy of this format using the specified output line separator
>       */
> +    public CSVFormat withLineSeparator(final char lineSeparator) {
> +        return new CSVFormat(delimiter, encapsulator, commentStart, escape, ignoreSurroundingSpaces,
> +                ignoreEmptyLines, String.valueOf(lineSeparator), header);
> +    }
> +
> +    /**
> +     * Returns a copy of this format using the specified output line separator.
> +     *
> +     * @param lineSeparator
> +     *            the line separator to be used for output.
> +     *
> +     * @return A copy of this format using the specified output line separator
> +     */
>      public CSVFormat withLineSeparator(final String lineSeparator) {
>          return new CSVFormat(delimiter, encapsulator, commentStart, escape, ignoreSurroundingSpaces,
>                  ignoreEmptyLines, lineSeparator, header);
>
> Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java?rev=1397556&r1=1397555&r2=1397556&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java (original)
> +++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java Fri Oct 12 13:15:30 2012
> @@ -39,6 +39,10 @@ class Constants {
>
>      /** Undefined state for the lookahead char */
>      static final int UNDEFINED = -2;
> +
> +    /** According to RFC 4180, line breaks are delimited by CRLF */
> +    public static final String CRLF = EMPTY + CR + LF;

That looks very awkward; not particularly easy to read either.

Why not use:

public static final String CRLF = "\r\n";

> +
>
>
>  }
>
> 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=1397556&r1=1397555&r2=1397556&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 Fri Oct 12 13:15:30 2012
> @@ -17,6 +17,7 @@
>
>  package org.apache.commons.csv;
>
> +import static org.apache.commons.csv.Constants.CRLF;
>  import static org.junit.Assert.assertEquals;
>  import static org.junit.Assert.assertFalse;
>  import static org.junit.Assert.assertNotNull;
> @@ -34,7 +35,7 @@ public class CSVFormatTest {
>
>      @Test
>      public void testImmutalibity() {
> -        final CSVFormat format = new CSVFormat('!', '!', '!', '!', true, true, CSVFormat.CRLF, null);
> +        final CSVFormat format = new CSVFormat('!', '!', '!', '!', true, true, CRLF, null);
>
>          format.withDelimiter('?');
>          format.withEncapsulator('?');
> @@ -48,7 +49,7 @@ public class CSVFormatTest {
>          assertEquals('!', format.getEncapsulator());
>          assertEquals('!', format.getCommentStart());
>          assertEquals('!', format.getEscape());
> -        assertEquals(CSVFormat.CRLF, format.getLineSeparator());
> +        assertEquals(CRLF, format.getLineSeparator());
>
>          assertTrue(format.getIgnoreSurroundingSpaces());
>          assertTrue(format.getIgnoreEmptyLines());
> @@ -56,7 +57,7 @@ public class CSVFormatTest {
>
>      @Test
>      public void testMutators() {
> -        final CSVFormat format = new CSVFormat('!', '!', '!', '!', true, true, CSVFormat.CRLF, null);
> +        final CSVFormat format = new CSVFormat('!', '!', '!', '!', true, true, CRLF, null);
>
>          assertEquals('?', format.withDelimiter('?').getDelimiter());
>          assertEquals('?', format.withEncapsulator('?').getEncapsulator());
>
> Modified: commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
> URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java?rev=1397556&r1=1397555&r2=1397556&view=diff
> ==============================================================================
> --- commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java (original)
> +++ commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java Fri Oct 12 13:15:30 2012
> @@ -17,6 +17,8 @@
>
>  package org.apache.commons.csv;
>
> +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;
> @@ -307,7 +309,7 @@ public class CSVParserTest {
>
>
>          final CSVFormat format = CSVFormat.PRISTINE.withDelimiter(',').withEncapsulator('\'').withEscape('/')
> -                               .withIgnoreEmptyLines(true).withLineSeparator(CSVFormat.CRLF);
> +                               .withIgnoreEmptyLines(true).withLineSeparator(CRLF);
>
>          final CSVParser parser = new CSVParser(code, format);
>          final List<CSVRecord> records = parser.getRecords();
> @@ -337,7 +339,7 @@ public class CSVParserTest {
>
>
>          final CSVFormat format = CSVFormat.PRISTINE.withDelimiter(',').withEscape('/')
> -                .withIgnoreEmptyLines(true).withLineSeparator(CSVFormat.CRLF);
> +                .withIgnoreEmptyLines(true).withLineSeparator(CRLF);
>
>          final CSVParser parser = new CSVParser(code, format);
>          final List<CSVRecord> records = parser.getRecords();
> @@ -584,7 +586,7 @@ public class CSVParserTest {
>
>      @Test
>      public void testGetLineNumberWithLF() throws Exception {
> -        final CSVParser parser = new CSVParser("a\nb\nc", CSVFormat.DEFAULT.withLineSeparator("\n"));
> +        final CSVParser parser = new CSVParser("a\nb\nc", CSVFormat.DEFAULT.withLineSeparator(LF));
>
>          assertEquals(0, parser.getLineNumber());
>          assertNotNull(parser.getRecord());
> @@ -598,7 +600,7 @@ public class CSVParserTest {
>
>      @Test
>      public void testGetLineNumberWithCRLF() throws Exception {
> -        final CSVParser parser = new CSVParser("a\r\nb\r\nc", CSVFormat.DEFAULT.withLineSeparator(CSVFormat.CRLF));
> +        final CSVParser parser = new CSVParser("a\r\nb\r\nc", CSVFormat.DEFAULT.withLineSeparator(CRLF));
>
>          assertEquals(0, parser.getLineNumber());
>          assertNotNull(parser.getRecord());
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


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

Posted by Gary Gregory <ga...@gmail.com>.
On Fri, Oct 12, 2012 at 9:44 AM, sebb <se...@gmail.com> wrote:

> On 12 October 2012 14:15,  <gg...@apache.org> wrote:
> > Author: ggregory
> > Date: Fri Oct 12 13:15:30 2012
> > New Revision: 1397556
> >
> > URL: http://svn.apache.org/viewvc?rev=1397556&view=rev
> > Log:
> > More constants clean ups.
> >
> > Modified:
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
> >
> commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVFormatTest.java
> >
> commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.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=1397556&r1=1397555&r2=1397556&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
> Fri Oct 12 13:15:30 2012
> > @@ -18,7 +18,7 @@
> >  package org.apache.commons.csv;
> >
> >  import static org.apache.commons.csv.Constants.COMMA;
> > -import static org.apache.commons.csv.Constants.CR;
> > +import static org.apache.commons.csv.Constants.CRLF;
> >  import static org.apache.commons.csv.Constants.DOUBLE_QUOTE;
> >  import static org.apache.commons.csv.Constants.ESCAPE;
> >  import static org.apache.commons.csv.Constants.LF;
> > @@ -36,13 +36,8 @@ import java.io.StringWriter;
> >   */
> >  public class CSVFormat implements Serializable {
> >
> > -    private static final String LF_STR = "" + LF;
> > -
> >      private static final long serialVersionUID = 1L;
> >
> > -    /** According to RFC 4180, line breaks are delimited by CRLF */
> > -    public static final String CRLF = "" + CR + LF;
> > -
> >      private final char delimiter;
> >      private final char encapsulator;
> >      private final char commentStart;
> > @@ -136,7 +131,7 @@ public class CSVFormat implements Serial
> >              PRISTINE
> >              .withDelimiter(TAB)
> >              .withEscape(ESCAPE)
> > -            .withLineSeparator(LF_STR);
> > +            .withLineSeparator(LF);
> >
> >      /**
> >       * Creates a customized CSV format.
> > @@ -413,6 +408,19 @@ public class CSVFormat implements Serial
> >       *
> >       * @return A copy of this format using the specified output line
> separator
> >       */
> > +    public CSVFormat withLineSeparator(final char lineSeparator) {
> > +        return new CSVFormat(delimiter, encapsulator, commentStart,
> escape, ignoreSurroundingSpaces,
> > +                ignoreEmptyLines, String.valueOf(lineSeparator),
> header);
> > +    }
> > +
> > +    /**
> > +     * Returns a copy of this format using the specified output line
> separator.
> > +     *
> > +     * @param lineSeparator
> > +     *            the line separator to be used for output.
> > +     *
> > +     * @return A copy of this format using the specified output line
> separator
> > +     */
> >      public CSVFormat withLineSeparator(final String lineSeparator) {
> >          return new CSVFormat(delimiter, encapsulator, commentStart,
> escape, ignoreSurroundingSpaces,
> >                  ignoreEmptyLines, lineSeparator, header);
> >
> > Modified:
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
> > URL:
> http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java?rev=1397556&r1=1397555&r2=1397556&view=diff
> >
> ==============================================================================
> > ---
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
> (original)
> > +++
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/Constants.java
> Fri Oct 12 13:15:30 2012
> > @@ -39,6 +39,10 @@ class Constants {
> >
> >      /** Undefined state for the lookahead char */
> >      static final int UNDEFINED = -2;
> > +
> > +    /** According to RFC 4180, line breaks are delimited by CRLF */
> > +    public static final String CRLF = EMPTY + CR + LF;
>
> That looks very awkward; not particularly easy to read either.
>
> Why not use:
>
> public static final String CRLF = "\r\n";
>

Done.

Gary


>
> > +
> >
> >
> >  }
> >
> > 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=1397556&r1=1397555&r2=1397556&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
> Fri Oct 12 13:15:30 2012
> > @@ -17,6 +17,7 @@
> >
> >  package org.apache.commons.csv;
> >
> > +import static org.apache.commons.csv.Constants.CRLF;
> >  import static org.junit.Assert.assertEquals;
> >  import static org.junit.Assert.assertFalse;
> >  import static org.junit.Assert.assertNotNull;
> > @@ -34,7 +35,7 @@ public class CSVFormatTest {
> >
> >      @Test
> >      public void testImmutalibity() {
> > -        final CSVFormat format = new CSVFormat('!', '!', '!', '!',
> true, true, CSVFormat.CRLF, null);
> > +        final CSVFormat format = new CSVFormat('!', '!', '!', '!',
> true, true, CRLF, null);
> >
> >          format.withDelimiter('?');
> >          format.withEncapsulator('?');
> > @@ -48,7 +49,7 @@ public class CSVFormatTest {
> >          assertEquals('!', format.getEncapsulator());
> >          assertEquals('!', format.getCommentStart());
> >          assertEquals('!', format.getEscape());
> > -        assertEquals(CSVFormat.CRLF, format.getLineSeparator());
> > +        assertEquals(CRLF, format.getLineSeparator());
> >
> >          assertTrue(format.getIgnoreSurroundingSpaces());
> >          assertTrue(format.getIgnoreEmptyLines());
> > @@ -56,7 +57,7 @@ public class CSVFormatTest {
> >
> >      @Test
> >      public void testMutators() {
> > -        final CSVFormat format = new CSVFormat('!', '!', '!', '!',
> true, true, CSVFormat.CRLF, null);
> > +        final CSVFormat format = new CSVFormat('!', '!', '!', '!',
> true, true, CRLF, null);
> >
> >          assertEquals('?', format.withDelimiter('?').getDelimiter());
> >          assertEquals('?',
> format.withEncapsulator('?').getEncapsulator());
> >
> > Modified:
> commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
> > URL:
> http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java?rev=1397556&r1=1397555&r2=1397556&view=diff
> >
> ==============================================================================
> > ---
> commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
> (original)
> > +++
> commons/proper/csv/trunk/src/test/java/org/apache/commons/csv/CSVParserTest.java
> Fri Oct 12 13:15:30 2012
> > @@ -17,6 +17,8 @@
> >
> >  package org.apache.commons.csv;
> >
> > +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;
> > @@ -307,7 +309,7 @@ public class CSVParserTest {
> >
> >
> >          final CSVFormat format =
> CSVFormat.PRISTINE.withDelimiter(',').withEncapsulator('\'').withEscape('/')
> > -
> .withIgnoreEmptyLines(true).withLineSeparator(CSVFormat.CRLF);
> > +
> .withIgnoreEmptyLines(true).withLineSeparator(CRLF);
> >
> >          final CSVParser parser = new CSVParser(code, format);
> >          final List<CSVRecord> records = parser.getRecords();
> > @@ -337,7 +339,7 @@ public class CSVParserTest {
> >
> >
> >          final CSVFormat format =
> CSVFormat.PRISTINE.withDelimiter(',').withEscape('/')
> > -
>  .withIgnoreEmptyLines(true).withLineSeparator(CSVFormat.CRLF);
> > +                .withIgnoreEmptyLines(true).withLineSeparator(CRLF);
> >
> >          final CSVParser parser = new CSVParser(code, format);
> >          final List<CSVRecord> records = parser.getRecords();
> > @@ -584,7 +586,7 @@ public class CSVParserTest {
> >
> >      @Test
> >      public void testGetLineNumberWithLF() throws Exception {
> > -        final CSVParser parser = new CSVParser("a\nb\nc",
> CSVFormat.DEFAULT.withLineSeparator("\n"));
> > +        final CSVParser parser = new CSVParser("a\nb\nc",
> CSVFormat.DEFAULT.withLineSeparator(LF));
> >
> >          assertEquals(0, parser.getLineNumber());
> >          assertNotNull(parser.getRecord());
> > @@ -598,7 +600,7 @@ public class CSVParserTest {
> >
> >      @Test
> >      public void testGetLineNumberWithCRLF() throws Exception {
> > -        final CSVParser parser = new CSVParser("a\r\nb\r\nc",
> CSVFormat.DEFAULT.withLineSeparator(CSVFormat.CRLF));
> > +        final CSVParser parser = new CSVParser("a\r\nb\r\nc",
> CSVFormat.DEFAULT.withLineSeparator(CRLF));
> >
> >          assertEquals(0, parser.getLineNumber());
> >          assertNotNull(parser.getRecord());
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@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