commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <benerit...@gmail.com>
Subject Re: svn commit: r1411919 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.java
Date Wed, 21 Nov 2012 07:44:25 GMT
2012/11/21 Gary Gregory <garydgregory@gmail.com>

> On Tue, Nov 20, 2012 at 6:22 PM, <sebb@apache.org> wrote:
>
> > Author: sebb
> > Date: Tue Nov 20 23:22:21 2012
> > New Revision: 1411919
> >
> > URL: http://svn.apache.org/viewvc?rev=1411919&view=rev
> > Log:
> > Make some methods package-protected to avoid the need for synthetic
> > accessors.
> > TODO consider whether to do so for the fields as well
> >
>
> How about just using getters and keeping encapsulation cleaner?
>

I agree.


>
> Gary
>
>
> >
> > Modified:
> >
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVFormat.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=1411919&r1=1411918&r2=1411919&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
> > Tue Nov 20 23:22:21 2012
> > @@ -175,9 +175,9 @@ 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)
> > +    // package protected to give access without needing a synthetic
> > accessor
> > +    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))
> >          {
> > @@ -202,7 +202,8 @@ public class CSVFormat implements Serial
> >       *
> >       * @return true if <code>c</code> is a line break character
> >       */
> > -    private static boolean isLineBreak(final Character c) {
> > +    // package protected to give access without needing a synthetic
> > accessor
> > +    static boolean isLineBreak(final Character c) {
> >          return c != null && isLineBreak(c.charValue());
> >      }
>

I still don't know where this methods belong to. They are defined in
CSVFormat and used in the builder and in CSVFormat ctor. I think the call
to isLineBreak could be moved to build() to get rid of that redundancy.
OTH if we need the method in other classes, maybe it makes sense to define
a package private util class, say CSVCharacters or CSVSymbols.

Benedikt


> >
> > @@ -214,7 +215,8 @@ public class CSVFormat implements Serial
> >       *
> >       * @return true if <code>c</code> is a line break character
> >       */
> > -    private static boolean isLineBreak(final char c) {
> > +    // package protected to give access without needing a synthetic
> > accessor
> > +    static boolean isLineBreak(final char c) {
> >          return c == LF || c == CR;
> >      }
> >
> > @@ -539,7 +541,9 @@ public class CSVFormat implements Serial
> >           * @param format
> >           *            The format to use values from
> >           */
> > -        private CSVFormatBuilder(CSVFormat format) {
> > +        @SuppressWarnings("synthetic-access") // TODO fields could be
> > made package-protected
> > +        // package protected to give access without needing a synthetic
> > accessor
> > +        CSVFormatBuilder(CSVFormat format) {
> >              this(format.delimiter, format.quoteChar, format.quotePolicy,
> >                      format.commentStart, format.escape,
> >                      format.ignoreSurroundingSpaces,
> > format.ignoreEmptyLines,
> > @@ -553,7 +557,8 @@ public class CSVFormat implements Serial
> >           *            the char used for value separation, must not be a
> > line break character
> >           * @throws IllegalArgumentException if the delimiter is a line
> > break character
> >           */
> > -        private CSVFormatBuilder(final char delimiter){
> > +        // package protected to give access without needing a synthetic
> > accessor
> > +        CSVFormatBuilder(final char delimiter){
> >              this(delimiter, null, null, null, null, false, false, null,
> > null);
> >          }
> >
> >
> >
> >
>
>
> --
> 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
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message