commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@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 13:45:50 GMT
On 21 November 2012 07:44, Benedikt Ritter <beneritter@gmail.com> wrote:
> 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.
>

The synthetic accessors are getters.

>>
>> 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
>>

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


Mime
View raw message