You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Yonik Seeley <yo...@apache.org> on 2006/12/07 20:56:43 UTC
[CSV] commit-then-review
FYI, for some minor changes to CSV, I'm using the commit-then-review
protocol, esp since there don't seem to be many CSV followers. Holler
if that doesn't work for you :-)
I deprecated setStrategy() as it doesn't make much sense... I don't
think we want to guarantee the ability to switch strategies in the
middle of parsing a CSV input, and setting at the beginning can (and
should) now be done via the constructor.
-Yonik
---------- Forwarded message ----------
From: yonik@apache.org <yo...@apache.org>
Date: Dec 7, 2006 1:05 PM
Subject: svn commit: r483576 -
/jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
To: commons-cvs@jakarta.apache.org
Author: yonik
Date: Thu Dec 7 10:05:00 2006
New Revision: 483576
URL: http://svn.apache.org/viewvc?view=rev&rev=483576
Log:
[CSV] deprecated setStrategy, made most private members final
Modified:
jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
Modified: jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
URL: http://svn.apache.org/viewvc/jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java?view=diff&rev=483576&r1=483575&r2=483576
==============================================================================
--- jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
(original)
+++ jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
Thu Dec 7 10:05:00 2006
@@ -69,16 +69,17 @@
private static final String[] EMPTY_STRING_ARRAY = new String[0];
// the input stream
- private ExtendedBufferedReader in;
+ private final ExtendedBufferedReader in;
+ // TODO: this can be made final if setStrategy is removed
private CSVStrategy strategy;
// the following objects are shared to reduce garbage
/** A record buffer for getLine(). Grows as necessary and is reused. */
- private ArrayList record = new ArrayList();
- private Token reusableToken = new Token();
- private CharBuffer wsBuf = new CharBuffer();
- private CharBuffer code = new CharBuffer(4);
+ private final ArrayList record = new ArrayList();
+ private final Token reusableToken = new Token();
+ private final CharBuffer wsBuf = new CharBuffer();
+ private final CharBuffer code = new CharBuffer(4);
/**
@@ -567,6 +568,7 @@
* Sets the specified CSV Strategy
*
* @return current instance of CSVParser to allow chained method calls
+ * @deprecated
*/
public CSVParser setStrategy(CSVStrategy strategy) {
this.strategy = strategy;
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [CSV] commit-then-review
Posted by Yonik Seeley <yo...@apache.org>.
On 12/7/06, Rahul Akolkar <ra...@gmail.com> wrote:
> On 12/7/06, Yonik Seeley <yo...@apache.org> wrote:
> > FYI, for some minor changes to CSV, I'm using the commit-then-review
> > protocol, esp since there don't seem to be many CSV followers. Holler
> > if that doesn't work for you :-)
> >
> > I deprecated setStrategy() as it doesn't make much sense... I don't
> > think we want to guarantee the ability to switch strategies in the
> > middle of parsing a CSV input, and setting at the beginning can (and
> > should) now be done via the constructor.
> >
> <snip/>
>
> Maybe add a line after the @deprecated tag as to why it is deprecated
> (or what the replacement is)?
Right, will do.
> And if it needs to go altogether, being in the sandbox is a good
> thing. Its probably odd, IMO, to find deprecated items in the first
> release.
Right... the idea was to remove the deprecated items before the first release.
-Yonik
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [CSV] commit-then-review
Posted by Rahul Akolkar <ra...@gmail.com>.
On 12/7/06, Yonik Seeley <yo...@apache.org> wrote:
> FYI, for some minor changes to CSV, I'm using the commit-then-review
> protocol, esp since there don't seem to be many CSV followers. Holler
> if that doesn't work for you :-)
>
> I deprecated setStrategy() as it doesn't make much sense... I don't
> think we want to guarantee the ability to switch strategies in the
> middle of parsing a CSV input, and setting at the beginning can (and
> should) now be done via the constructor.
>
<snip/>
Maybe add a line after the @deprecated tag as to why it is deprecated
(or what the replacement is)?
And if it needs to go altogether, being in the sandbox is a good
thing. Its probably odd, IMO, to find deprecated items in the first
release.
-Rahul
> -Yonik
>
> ---------- Forwarded message ----------
> From: yonik@apache.org <yo...@apache.org>
> Date: Dec 7, 2006 1:05 PM
> Subject: svn commit: r483576 -
> /jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> To: commons-cvs@jakarta.apache.org
>
>
> Author: yonik
> Date: Thu Dec 7 10:05:00 2006
> New Revision: 483576
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=483576
> Log:
> [CSV] deprecated setStrategy, made most private members final
>
> Modified:
> jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
>
> Modified: jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> URL: http://svn.apache.org/viewvc/jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java?view=diff&rev=483576&r1=483575&r2=483576
> ==============================================================================
> --- jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> (original)
> +++ jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> Thu Dec 7 10:05:00 2006
> @@ -69,16 +69,17 @@
> private static final String[] EMPTY_STRING_ARRAY = new String[0];
>
> // the input stream
> - private ExtendedBufferedReader in;
> + private final ExtendedBufferedReader in;
>
> + // TODO: this can be made final if setStrategy is removed
> private CSVStrategy strategy;
>
> // the following objects are shared to reduce garbage
> /** A record buffer for getLine(). Grows as necessary and is reused. */
> - private ArrayList record = new ArrayList();
> - private Token reusableToken = new Token();
> - private CharBuffer wsBuf = new CharBuffer();
> - private CharBuffer code = new CharBuffer(4);
> + private final ArrayList record = new ArrayList();
> + private final Token reusableToken = new Token();
> + private final CharBuffer wsBuf = new CharBuffer();
> + private final CharBuffer code = new CharBuffer(4);
>
>
> /**
> @@ -567,6 +568,7 @@
> * Sets the specified CSV Strategy
> *
> * @return current instance of CSVParser to allow chained method calls
> + * @deprecated
> */
> public CSVParser setStrategy(CSVStrategy strategy) {
> this.strategy = strategy;
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [CSV] commit-then-review
Posted by Henri Yandell <fl...@gmail.com>.
+1.
I favour making JIRA items for anything substantial - then we can use
it to create a nice set of release notes etc. [and now I'm hoping that
I've been doing that when I've made substantial changes :) ].
It's also a much better focal point than the code changes themselves.
Hen
On 12/7/06, Yonik Seeley <yo...@apache.org> wrote:
> FYI, for some minor changes to CSV, I'm using the commit-then-review
> protocol, esp since there don't seem to be many CSV followers. Holler
> if that doesn't work for you :-)
>
> I deprecated setStrategy() as it doesn't make much sense... I don't
> think we want to guarantee the ability to switch strategies in the
> middle of parsing a CSV input, and setting at the beginning can (and
> should) now be done via the constructor.
>
> -Yonik
>
> ---------- Forwarded message ----------
> From: yonik@apache.org <yo...@apache.org>
> Date: Dec 7, 2006 1:05 PM
> Subject: svn commit: r483576 -
> /jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> To: commons-cvs@jakarta.apache.org
>
>
> Author: yonik
> Date: Thu Dec 7 10:05:00 2006
> New Revision: 483576
>
> URL: http://svn.apache.org/viewvc?view=rev&rev=483576
> Log:
> [CSV] deprecated setStrategy, made most private members final
>
> Modified:
> jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
>
> Modified: jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> URL: http://svn.apache.org/viewvc/jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java?view=diff&rev=483576&r1=483575&r2=483576
> ==============================================================================
> --- jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> (original)
> +++ jakarta/commons/sandbox/csv/trunk/src/java/org/apache/commons/csv/CSVParser.java
> Thu Dec 7 10:05:00 2006
> @@ -69,16 +69,17 @@
> private static final String[] EMPTY_STRING_ARRAY = new String[0];
>
> // the input stream
> - private ExtendedBufferedReader in;
> + private final ExtendedBufferedReader in;
>
> + // TODO: this can be made final if setStrategy is removed
> private CSVStrategy strategy;
>
> // the following objects are shared to reduce garbage
> /** A record buffer for getLine(). Grows as necessary and is reused. */
> - private ArrayList record = new ArrayList();
> - private Token reusableToken = new Token();
> - private CharBuffer wsBuf = new CharBuffer();
> - private CharBuffer code = new CharBuffer(4);
> + private final ArrayList record = new ArrayList();
> + private final Token reusableToken = new Token();
> + private final CharBuffer wsBuf = new CharBuffer();
> + private final CharBuffer code = new CharBuffer(4);
>
>
> /**
> @@ -567,6 +568,7 @@
> * Sets the specified CSV Strategy
> *
> * @return current instance of CSVParser to allow chained method calls
> + * @deprecated
> */
> public CSVParser setStrategy(CSVStrategy strategy) {
> this.strategy = strategy;
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org