Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: domain of sebbaz@gmail.com designates
 209.85.220.171 as permitted sender)
MIME-Version: 1.0
In-Reply-To: 
 <CACZkXPySgT3EhgJhfATBM0OY8Vw8GCJX=se5PwbYhk43sbHukg@mail.gmail.com>
References: <20121012121244.AC36C23888CD@eris.apache.org>
	<CAB917RLXUBru8go=7gyyPS9eOni1+CfQ7MsyqfQNR9jO7Qe0Vg@mail.gmail.com>
	<CACZkXPw-L-YPgy+GJT7b=wW34nrvkbghFB-1DRxW-_OG=xdvfw@mail.gmail.com>
	<CAOGo0VYFXURfv+F_TVQx-4Z8GffJGuHAhihrQqh3vt6fwTfuTA@mail.gmail.com>
	<CACZkXPySgT3EhgJhfATBM0OY8Vw8GCJX=se5PwbYhk43sbHukg@mail.gmail.com>
Date: Fri, 12 Oct 2012 14:56:29 +0100
Message-ID: 
 <CAOGo0VZV6+LABf3SeCE83do3rFbuAkJz+k84VT9qtsZ92in5zQ@mail.gmail.com>
Subject: Re: svn commit: r1397534 -
 /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java
From: sebb <sebbaz@gmail.com>
To: Commons Developers List <dev@commons.apache.org>
Content-Type: text/plain; charset=ISO-8859-1

On 12 October 2012 14:50, Gary Gregory <garydgregory@gmail.com> wrote:
> On Fri, Oct 12, 2012 at 9:39 AM, sebb <sebbaz@gmail.com> wrote:
>
>> On 12 October 2012 13:38, Gary Gregory <garydgregory@gmail.com> wrote:
>> > On Fri, Oct 12, 2012 at 8:22 AM, Benedikt Ritter <beneritter@gmail.com
>> >wrote:
>> >
>> >> Hi
>> >>
>> >> 2012/10/12  <ggregory@apache.org>:
>> >> > Author: ggregory
>> >> > Date: Fri Oct 12 12:12:44 2012
>> >> > New Revision: 1397534
>> >> >
>> >> > URL: http://svn.apache.org/viewvc?rev=1397534&view=rev
>> >> > Log:
>> >> > Refactor magic strings into constants.
>> >> >
>> >> > Modified:
>> >> >
>> >>
>> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVPrinter.java
>> >> >
>> >> > 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=1397534&r1=1397533&r2=1397534&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
>> >> Fri Oct 12 12:12:44 2012
>> >> > @@ -25,6 +25,12 @@ import java.io.IOException;
>> >> >   */
>> >> >  public class CSVPrinter {
>> >> >
>> >> > +    private static final char COMMENT = '#';
>> >>
>> >> How about COMMENT_START ?
>> >>
>> >
>> > I would say yes only /if/ there were a COMMENT_END.
>>
>> INLINE_COMMENT_INTRODUCER ?
>>
>
> IS_IT_APRIL_1?

No.

But I agree it's not an ideal name.

It's just that COMMENT on its own is not ideal either.

Maybe the solution is to add Javadoc to explain how the # is used to
introduce comments.


> I do not know you well enough to read you ;)
>
> G
>
>
>> > Gary
>> >
>> >
>> >> Benedikt
>> >>
>> >> > +    private static final String EMPTY = "";
>> >> > +    private static final char SP = ' ';
>> >> > +    private static final char CR = '\r';
>> >> > +    private static final char LF = '\n';
>> >> > +
>> >> >      /** The place that the values get written. */
>> >> >      private final Appendable out;
>> >> >      private final CSVFormat format;
>> >> > @@ -106,19 +112,19 @@ public class CSVPrinter {
>> >> >              println();
>> >> >          }
>> >> >          out.append(format.getCommentStart());
>> >> > -        out.append(' ');
>> >> > +        out.append(SP);
>> >> >          for (int i = 0; i < comment.length(); i++) {
>> >> >              final char c = comment.charAt(i);
>> >> >              switch (c) {
>> >> > -            case '\r':
>> >> > -                if (i + 1 < comment.length() && comment.charAt(i + 1)
>> >> == '\n') {
>> >> > +            case CR:
>> >> > +                if (i + 1 < comment.length() && comment.charAt(i + 1)
>> >> == LF) {
>> >> >                      i++;
>> >> >                  }
>> >> >                  //$FALL-THROUGH$ break intentionally excluded.
>> >> > -            case '\n':
>> >> > +            case LF:
>> >> >                  println();
>> >> >                  out.append(format.getCommentStart());
>> >> > -                out.append(' ');
>> >> > +                out.append(SP);
>> >> >                  break;
>> >> >              default:
>> >> >                  out.append(c);
>> >> > @@ -159,14 +165,14 @@ public class CSVPrinter {
>> >> >
>> >> >          while (pos < end) {
>> >> >              char c = value.charAt(pos);
>> >> > -            if (c == '\r' || c == '\n' || c == delim || c == escape)
>> {
>> >> > +            if (c == CR || c == LF || c == delim || c == escape) {
>> >> >                  // write out segment up until this char
>> >> >                  if (pos > start) {
>> >> >                      out.append(value, start, pos);
>> >> >                  }
>> >> > -                if (c == '\n') {
>> >> > +                if (c == LF) {
>> >> >                      c = 'n';
>> >> > -                } else if (c == '\r') {
>> >> > +                } else if (c == CR) {
>> >> >                      c = 'r';
>> >> >                  }
>> >> >
>> >> > @@ -212,7 +218,7 @@ public class CSVPrinter {
>> >> >              if (first && (c < '0' || (c > '9' && c < 'A') || (c > 'Z'
>> >> && c < 'a') || (c > 'z'))) {
>> >> >                  quote = true;
>> >> >                  // } else if (c == ' ' || c == '\f' || c == '\t') {
>> >> > -            } else if (c <= '#') {
>> >> > +            } else if (c <= COMMENT) {
>> >> >                  // Some other chars at the start of a value caused
>> the
>> >> parser to fail, so for now
>> >> >                  // encapsulate if we start in anything less than '#'.
>> >> We are being conservative
>> >> >                  // by including the default comment char too.
>> >> > @@ -220,7 +226,7 @@ public class CSVPrinter {
>> >> >              } else {
>> >> >                  while (pos < end) {
>> >> >                      c = value.charAt(pos);
>> >> > -                    if (c == '\n' || c == '\r' || c == encapsulator
>> ||
>> >> c == delim) {
>> >> > +                    if (c == LF || c == CR || c == encapsulator || c
>> ==
>> >> delim) {
>> >> >                          quote = true;
>> >> >                          break;
>> >> >                      }
>> >> > @@ -233,7 +239,7 @@ public class CSVPrinter {
>> >> >                      // if (c == ' ' || c == '\f' || c == '\t') {
>> >> >                      // Some other chars at the end caused the parser
>> to
>> >> fail, so for now
>> >> >                      // encapsulate if we end in anything less than '
>> '
>> >> > -                    if (c <= ' ') {
>> >> > +                    if (c <= SP) {
>> >> >                          quote = true;
>> >> >                      }
>> >> >                  }
>> >> > @@ -280,7 +286,7 @@ public class CSVPrinter {
>> >> >      public void print(String value, final boolean checkForEscape)
>> >> throws IOException {
>> >> >          if (value == null) {
>> >> >              // null values are considered empty
>> >> > -            value = "";
>> >> > +            value = EMPTY;
>> >> >          }
>> >> >
>> >> >          if (!checkForEscape) {
>> >> >
>> >> >
>> >>
>> >> ---------------------------------------------------------------------
>> >> 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
>>
>> ---------------------------------------------------------------------
>> 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

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