Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java

garydgregory
Doesn't the compiler do this already? 

Also please try not to mix in other changes like format changes in a single commit.

Gary

<div>-------- Original message --------</div><div>From: [hidden email] </div><div>Date:09/10/2014  13:28  (GMT-05:00) </div><div>To: [hidden email] </div><div>Subject: svn commit: r1624061 -  /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java </div><div>
</div>Author: brentworden
Date: Wed Sep 10 17:28:26 2014
New Revision: 1624061

URL: http://svn.apache.org/r1624061
Log:
CSV-124 replace string concatenation with StringBuilder

Modified:
    commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java

Modified: commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
URL: http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java?rev=1624061&r1=1624060&r2=1624061&view=diff
==============================================================================
--- commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java (original)
+++ commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java Wed Sep 10 17:28:26 2014
@@ -48,8 +48,7 @@ public final class CSVRecord implements
     /** The values of the record */
     private final String[] values;

-    CSVRecord(final String[] values, final Map<String, Integer> mapping,
-            final String comment, final long recordNumber) {
+    CSVRecord(final String[] values, final Map<String, Integer> mapping, final String comment, final long recordNumber) {
         this.recordNumber = recordNumber;
         this.values = values != null ? values : EMPTY_STRING_ARRAY;
         this.mapping = mapping;
@@ -94,27 +93,26 @@ public final class CSVRecord implements
     public String get(final String name) {
         if (mapping == null) {
             throw new IllegalStateException(
-                    "No header mapping was specified, the record values can't be accessed by name");
+                "No header mapping was specified, the record values can't be accessed by name");
         }
         final Integer index = mapping.get(name);
         if (index == null) {
             throw new IllegalArgumentException(String.format("Mapping for %s not found, expected one of %s", name,
-                    mapping.keySet()));
+                mapping.keySet()));
         }
         try {
             return values[index.intValue()];
         } catch (final ArrayIndexOutOfBoundsException e) {
             throw new IllegalArgumentException(String.format(
-                    "Index for header '%s' is %d but CSVRecord only has %d values!", name, index,
-                    Integer.valueOf(values.length)));
+                "Index for header '%s' is %d but CSVRecord only has %d values!", name, index,
+                Integer.valueOf(values.length)));
         }
     }

     /**
      * Returns the comment for this record, if any.
      *
-     * @return the comment for this record, or null if no comment for this
-     *         record is available.
+     * @return the comment for this record, or null if no comment for this record is available.
      */
     public String getComment() {
         return comment;
@@ -176,6 +174,7 @@ public final class CSVRecord implements
      *
      * @return an iterator over the values of this record.
      */
+    @Override
     public Iterator<String> iterator() {
         return toList().iterator();
     }
@@ -183,7 +182,8 @@ public final class CSVRecord implements
     /**
      * Puts all values of this record into the given Map.
      *
-     * @param map The Map to populate.
+     * @param map
+     *            The Map to populate.
      * @return the given map.
      */
     <M extends Map<String, String>> M putIn(final M map) {
@@ -212,6 +212,7 @@ public final class CSVRecord implements
      * Converts the values to a List.
      *
      * TODO: Maybe make this public?
+     *
      * @return a new List
      */
     private List<String> toList() {
@@ -227,7 +228,6 @@ public final class CSVRecord implements
         return putIn(new HashMap<String, String>(values.length));
     }

-
     /**
      * Returns a string representation of the contents of this record. The result is constructed by comment, mapping,
      * recordNumber and by passing the internal values array to {@link Arrays#toString(Object[])}.
@@ -236,14 +236,16 @@ public final class CSVRecord implements
      */
     @Override
     public String toString() {
-        return "CSVRecord [comment=" + comment + ", mapping=" + mapping +
-                ", recordNumber=" + recordNumber + ", values=" +
-                Arrays.toString(values) + "]";
+        StringBuilder sb = new StringBuilder();
+        sb.append("CSVRecord [comment=").append(comment);
+        sb.append(", mapping=").append(mapping);
+        sb.append(", recordNumber=").append(recordNumber);
+        sb.append(", values=").append(Arrays.toString(values)).append(']');
+        return sb.toString();
     }

     String[] values() {
         return values;
     }

-
}


Reply | Threaded
Open this post in threaded view
|

Re: Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java

Brent Worden-2
Examining the bytecode, it apparently does.  At least for JDK 1.7 anyway.
 I reverted the change.

Apologies for the reformatting.  I had my IDE configured to auto-format
files on save and I committed without checking the diff.

Thanks,

Brent

On Wed, Sep 10, 2014 at 2:27 PM, Gary Gregory <[hidden email]>
wrote:

> Doesn't the compiler do this already?
>
> Also please try not to mix in other changes like format changes in a
> single commit.
>
> Gary
>
> <div>-------- Original message --------</div><div>From:
> [hidden email] </div><div>Date:09/10/2014  13:28  (GMT-05:00)
> </div><div>To: [hidden email] </div><div>Subject: svn commit:
> r1624061 -
>
> /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> </div><div>
> </div>Author: brentworden
> Date: Wed Sep 10 17:28:26 2014
> New Revision: 1624061
>
> URL: http://svn.apache.org/r1624061
> Log:
> CSV-124 replace string concatenation with StringBuilder
>
> Modified:
>
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
>
> Modified:
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> URL:
> http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java?rev=1624061&r1=1624060&r2=1624061&view=diff
>
> ==============================================================================
> ---
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> (original)
> +++
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> Wed Sep 10 17:28:26 2014
> @@ -48,8 +48,7 @@ public final class CSVRecord implements
>      /** The values of the record */
>      private final String[] values;
>
> -    CSVRecord(final String[] values, final Map<String, Integer> mapping,
> -            final String comment, final long recordNumber) {
> +    CSVRecord(final String[] values, final Map<String, Integer> mapping,
> final String comment, final long recordNumber) {
>          this.recordNumber = recordNumber;
>          this.values = values != null ? values : EMPTY_STRING_ARRAY;
>          this.mapping = mapping;
> @@ -94,27 +93,26 @@ public final class CSVRecord implements
>      public String get(final String name) {
>          if (mapping == null) {
>              throw new IllegalStateException(
> -                    "No header mapping was specified, the record values
> can't be accessed by name");
> +                "No header mapping was specified, the record values can't
> be accessed by name");
>          }
>          final Integer index = mapping.get(name);
>          if (index == null) {
>              throw new IllegalArgumentException(String.format("Mapping for
> %s not found, expected one of %s", name,
> -                    mapping.keySet()));
> +                mapping.keySet()));
>          }
>          try {
>              return values[index.intValue()];
>          } catch (final ArrayIndexOutOfBoundsException e) {
>              throw new IllegalArgumentException(String.format(
> -                    "Index for header '%s' is %d but CSVRecord only has
> %d values!", name, index,
> -                    Integer.valueOf(values.length)));
> +                "Index for header '%s' is %d but CSVRecord only has %d
> values!", name, index,
> +                Integer.valueOf(values.length)));
>          }
>      }
>
>      /**
>       * Returns the comment for this record, if any.
>       *
> -     * @return the comment for this record, or null if no comment for this
> -     *         record is available.
> +     * @return the comment for this record, or null if no comment for
> this record is available.
>       */
>      public String getComment() {
>          return comment;
> @@ -176,6 +174,7 @@ public final class CSVRecord implements
>       *
>       * @return an iterator over the values of this record.
>       */
> +    @Override
>      public Iterator<String> iterator() {
>          return toList().iterator();
>      }
> @@ -183,7 +182,8 @@ public final class CSVRecord implements
>      /**
>       * Puts all values of this record into the given Map.
>       *
> -     * @param map The Map to populate.
> +     * @param map
> +     *            The Map to populate.
>       * @return the given map.
>       */
>      <M extends Map<String, String>> M putIn(final M map) {
> @@ -212,6 +212,7 @@ public final class CSVRecord implements
>       * Converts the values to a List.
>       *
>       * TODO: Maybe make this public?
> +     *
>       * @return a new List
>       */
>      private List<String> toList() {
> @@ -227,7 +228,6 @@ public final class CSVRecord implements
>          return putIn(new HashMap<String, String>(values.length));
>      }
>
> -
>      /**
>       * Returns a string representation of the contents of this record.
> The result is constructed by comment, mapping,
>       * recordNumber and by passing the internal values array to {@link
> Arrays#toString(Object[])}.
> @@ -236,14 +236,16 @@ public final class CSVRecord implements
>       */
>      @Override
>      public String toString() {
> -        return "CSVRecord [comment=" + comment + ", mapping=" + mapping +
> -                ", recordNumber=" + recordNumber + ", values=" +
> -                Arrays.toString(values) + "]";
> +        StringBuilder sb = new StringBuilder();
> +        sb.append("CSVRecord [comment=").append(comment);
> +        sb.append(", mapping=").append(mapping);
> +        sb.append(", recordNumber=").append(recordNumber);
> +        sb.append(",
> values=").append(Arrays.toString(values)).append(']');
> +        return sb.toString();
>      }
>
>      String[] values() {
>          return values;
>      }
>
> -
> }
>
>
>
Reply | Threaded
Open this post in threaded view
|

Re: Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java

garydgregory
Roger that. I think the Java compiler has been doing that for a long time.

Gary

On Thu, Sep 11, 2014 at 8:59 AM, Brent Worden <[hidden email]>
wrote:

> Examining the bytecode, it apparently does.  At least for JDK 1.7 anyway.
>  I reverted the change.
>
> Apologies for the reformatting.  I had my IDE configured to auto-format
> files on save and I committed without checking the diff.
>
> Thanks,
>
> Brent
>
> On Wed, Sep 10, 2014 at 2:27 PM, Gary Gregory <[hidden email]>
> wrote:
>
> > Doesn't the compiler do this already?
> >
> > Also please try not to mix in other changes like format changes in a
> > single commit.
> >
> > Gary
> >
> > <div>-------- Original message --------</div><div>From:
> > [hidden email] </div><div>Date:09/10/2014  13:28  (GMT-05:00)
> > </div><div>To: [hidden email] </div><div>Subject: svn
> commit:
> > r1624061 -
> >
> >
> /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> > </div><div>
> > </div>Author: brentworden
> > Date: Wed Sep 10 17:28:26 2014
> > New Revision: 1624061
> >
> > URL: http://svn.apache.org/r1624061
> > Log:
> > CSV-124 replace string concatenation with StringBuilder
> >
> > Modified:
> >
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> >
> > Modified:
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> > URL:
> >
> http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java?rev=1624061&r1=1624060&r2=1624061&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> > (original)
> > +++
> >
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
> > Wed Sep 10 17:28:26 2014
> > @@ -48,8 +48,7 @@ public final class CSVRecord implements
> >      /** The values of the record */
> >      private final String[] values;
> >
> > -    CSVRecord(final String[] values, final Map<String, Integer> mapping,
> > -            final String comment, final long recordNumber) {
> > +    CSVRecord(final String[] values, final Map<String, Integer> mapping,
> > final String comment, final long recordNumber) {
> >          this.recordNumber = recordNumber;
> >          this.values = values != null ? values : EMPTY_STRING_ARRAY;
> >          this.mapping = mapping;
> > @@ -94,27 +93,26 @@ public final class CSVRecord implements
> >      public String get(final String name) {
> >          if (mapping == null) {
> >              throw new IllegalStateException(
> > -                    "No header mapping was specified, the record values
> > can't be accessed by name");
> > +                "No header mapping was specified, the record values
> can't
> > be accessed by name");
> >          }
> >          final Integer index = mapping.get(name);
> >          if (index == null) {
> >              throw new IllegalArgumentException(String.format("Mapping
> for
> > %s not found, expected one of %s", name,
> > -                    mapping.keySet()));
> > +                mapping.keySet()));
> >          }
> >          try {
> >              return values[index.intValue()];
> >          } catch (final ArrayIndexOutOfBoundsException e) {
> >              throw new IllegalArgumentException(String.format(
> > -                    "Index for header '%s' is %d but CSVRecord only has
> > %d values!", name, index,
> > -                    Integer.valueOf(values.length)));
> > +                "Index for header '%s' is %d but CSVRecord only has %d
> > values!", name, index,
> > +                Integer.valueOf(values.length)));
> >          }
> >      }
> >
> >      /**
> >       * Returns the comment for this record, if any.
> >       *
> > -     * @return the comment for this record, or null if no comment for
> this
> > -     *         record is available.
> > +     * @return the comment for this record, or null if no comment for
> > this record is available.
> >       */
> >      public String getComment() {
> >          return comment;
> > @@ -176,6 +174,7 @@ public final class CSVRecord implements
> >       *
> >       * @return an iterator over the values of this record.
> >       */
> > +    @Override
> >      public Iterator<String> iterator() {
> >          return toList().iterator();
> >      }
> > @@ -183,7 +182,8 @@ public final class CSVRecord implements
> >      /**
> >       * Puts all values of this record into the given Map.
> >       *
> > -     * @param map The Map to populate.
> > +     * @param map
> > +     *            The Map to populate.
> >       * @return the given map.
> >       */
> >      <M extends Map<String, String>> M putIn(final M map) {
> > @@ -212,6 +212,7 @@ public final class CSVRecord implements
> >       * Converts the values to a List.
> >       *
> >       * TODO: Maybe make this public?
> > +     *
> >       * @return a new List
> >       */
> >      private List<String> toList() {
> > @@ -227,7 +228,6 @@ public final class CSVRecord implements
> >          return putIn(new HashMap<String, String>(values.length));
> >      }
> >
> > -
> >      /**
> >       * Returns a string representation of the contents of this record.
> > The result is constructed by comment, mapping,
> >       * recordNumber and by passing the internal values array to {@link
> > Arrays#toString(Object[])}.
> > @@ -236,14 +236,16 @@ public final class CSVRecord implements
> >       */
> >      @Override
> >      public String toString() {
> > -        return "CSVRecord [comment=" + comment + ", mapping=" + mapping
> +
> > -                ", recordNumber=" + recordNumber + ", values=" +
> > -                Arrays.toString(values) + "]";
> > +        StringBuilder sb = new StringBuilder();
> > +        sb.append("CSVRecord [comment=").append(comment);
> > +        sb.append(", mapping=").append(mapping);
> > +        sb.append(", recordNumber=").append(recordNumber);
> > +        sb.append(",
> > values=").append(Arrays.toString(values)).append(']');
> > +        return sb.toString();
> >      }
> >
> >      String[] values() {
> >          return values;
> >      }
> >
> > -
> > }
> >
> >
> >
>



--
E-Mail: [hidden email] | [hidden email]
Java Persistence with Hibernate, Second Edition
<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory