You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Gary Gregory <ga...@gmail.com> on 2014/09/10 21:27:17 UTC
Fwd: svn commit: r1624061 -
/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
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: brentworden@apache.org </div><div>Date:09/10/2014 13:28 (GMT-05:00) </div><div>To: commits@commons.apache.org </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;
}
-
}
Re: Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
Posted by Gary Gregory <ga...@gmail.com>.
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 <br...@gmail.com>
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 <ga...@gmail.com>
> 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:
> > brentworden@apache.org </div><div>Date:09/10/2014 13:28 (GMT-05:00)
> > </div><div>To: commits@commons.apache.org </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: garydgregory@gmail.com | ggregory@apache.org
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
Re: Fwd: svn commit: r1624061 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVRecord.java
Posted by Brent Worden <br...@gmail.com>.
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 <ga...@gmail.com>
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:
> brentworden@apache.org </div><div>Date:09/10/2014 13:28 (GMT-05:00)
> </div><div>To: commits@commons.apache.org </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;
> }
>
> -
> }
>
>
>