Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

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

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Gilles Sadowski
On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
> Repository: commons-csv
> Updated Branches:
>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>
>
> CSV-216: Avoid Arrays.copyOf()

Why?

> as .clone() will do

We should rather avoid using "clone()".

Gilles

> -- at least until someone tries to do
> .withValue(x) in an out-of-range column
>
>
> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
> Commit:
> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
> Tree:
> http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
> Diff:
> http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>
> Branch: refs/heads/CSV-216
> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
> Parents: 637ad2d
> Author: Stian Soiland-Reyes <[hidden email]>
> Authored: Fri Feb 9 16:49:51 2018 +0000
> Committer: Stian Soiland-Reyes <[hidden email]>
> Committed: Tue Feb 13 00:14:52 2018 +0000
>
>
> ----------------------------------------------------------------------
>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> ----------------------------------------------------------------------
>
>
>
> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>
> ----------------------------------------------------------------------
> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
> b/src/main/java/org/apache/commons/csv/CSVRecord.java
> index 979119f..2be5c49 100644
> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
> Iterable<String> {
>      public final CSVRecord immutable() {
>       if (isMutable()) {
>       // Subclass is probably CSVMutableRecord, freeze values
> -     String[] frozenValue = Arrays.copyOf(values, values.length);
> + String[] frozenValue = values.clone();
>       return new CSVRecord(frozenValue, mapping, comment,
> recordNumber, characterPosition);
>       } else {
>       return this;
> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
> Iterable<String> {
>       if (isMutable()) {
>       return this;
>       }
> - String[] newValues = Arrays.copyOf(values, values.length);
> + String[] newValues = values.clone();
>       return new CSVMutableRecord(newValues, mapping, comment,
> recordNumber, characterPosition);
>   }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

sebb-2-2
On 13 February 2018 at 09:31, Gilles <[hidden email]> wrote:

> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>
>> Repository: commons-csv
>> Updated Branches:
>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>
>>
>> CSV-216: Avoid Arrays.copyOf()
>
>
> Why?

Agreed

>> as .clone() will do
>
>
> We should rather avoid using "clone()".

Again: why?

It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
is considered bad.

> Gilles
>
>
>> -- at least until someone tries to do
>> .withValue(x) in an out-of-range column
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>> Commit: http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>> Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>> Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>
>> Branch: refs/heads/CSV-216
>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>> Parents: 637ad2d
>> Author: Stian Soiland-Reyes <[hidden email]>
>> Authored: Fri Feb 9 16:49:51 2018 +0000
>> Committer: Stian Soiland-Reyes <[hidden email]>
>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>
>>
>> ----------------------------------------------------------------------
>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> ----------------------------------------------------------------------
>>
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>
>> ----------------------------------------------------------------------
>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>> index 979119f..2be5c49 100644
>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>> Iterable<String> {
>>      public final CSVRecord immutable() {
>>         if (isMutable()) {
>>                 // Subclass is probably CSVMutableRecord, freeze values
>> -               String[] frozenValue = Arrays.copyOf(values,
>> values.length);
>> +               String[] frozenValue = values.clone();
>>                 return new CSVRecord(frozenValue, mapping, comment,
>> recordNumber, characterPosition);
>>         } else {
>>                 return this;
>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>> Iterable<String> {
>>         if (isMutable()) {
>>                 return this;
>>         }
>> -               String[] newValues = Arrays.copyOf(values, values.length);
>> +               String[] newValues = values.clone();
>>         return new CSVMutableRecord(newValues, mapping, comment,
>> recordNumber, characterPosition);
>>         }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Gilles Sadowski
On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:

> On 13 February 2018 at 09:31, Gilles <[hidden email]>
> wrote:
>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>>
>>> Repository: commons-csv
>>> Updated Branches:
>>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>
>>>
>>> CSV-216: Avoid Arrays.copyOf()
>>
>>
>> Why?
>
> Agreed
>
>>> as .clone() will do
>>
>>
>> We should rather avoid using "clone()".
>
> Again: why?

There are many discussions about this topic on the web.[1]
My point is that using "clone()" is not good advice.

Gilles

[1] E.g. http://vojtechruzicka.com/java-cloning-problems/

> It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
> is considered bad.
>
>
>> Gilles
>>
>>
>>> -- at least until someone tries to do
>>> .withValue(x) in an out-of-range column
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>> Commit:
>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>> Tree:
>>> http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>> Diff:
>>> http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>
>>> Branch: refs/heads/CSV-216
>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>> Parents: 637ad2d
>>> Author: Stian Soiland-Reyes <[hidden email]>
>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>
>>>
>>>
>>> ----------------------------------------------------------------------
>>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>>
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>
>>>
>>> ----------------------------------------------------------------------
>>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>> index 979119f..2be5c49 100644
>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>>> Iterable<String> {
>>>      public final CSVRecord immutable() {
>>>         if (isMutable()) {
>>>                 // Subclass is probably CSVMutableRecord, freeze
>>> values
>>> -               String[] frozenValue = Arrays.copyOf(values,
>>> values.length);
>>> +               String[] frozenValue = values.clone();
>>>                 return new CSVRecord(frozenValue, mapping, comment,
>>> recordNumber, characterPosition);
>>>         } else {
>>>                 return this;
>>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>>> Iterable<String> {
>>>         if (isMutable()) {
>>>                 return this;
>>>         }
>>> -               String[] newValues = Arrays.copyOf(values,
>>> values.length);
>>> +               String[] newValues = values.clone();
>>>         return new CSVMutableRecord(newValues, mapping, comment,
>>> recordNumber, characterPosition);
>>>         }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Stian Soiland-Reyes
I agree in general for .clone() on objects - and I'm trying to move
away from using .clone() in that Commons RDF fluent mutable/immutable
builder (See COMMONSRDF-49).

But this is an Object[] - a native type.

I must admit I am not sure what is really the preferred way - I
thought .clone() is best as the VM can copy an array natively however
it wants, while with Arrays.copyOf it might have to check if length is
the same before doing anything clever.

On 13 February 2018 at 18:58, Gilles <[hidden email]> wrote:

> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>
>> On 13 February 2018 at 09:31, Gilles <[hidden email]> wrote:
>>>
>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>>>
>>>>
>>>> Repository: commons-csv
>>>> Updated Branches:
>>>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>
>>>>
>>>> CSV-216: Avoid Arrays.copyOf()
>>>
>>>
>>>
>>> Why?
>>
>>
>> Agreed
>>
>>>> as .clone() will do
>>>
>>>
>>>
>>> We should rather avoid using "clone()".
>>
>>
>> Again: why?
>
>
> There are many discussions about this topic on the web.[1]
> My point is that using "clone()" is not good advice.
>
> Gilles
>
> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>
>
>> It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
>> is considered bad.
>>
>>
>>> Gilles
>>>
>>>
>>>> -- at least until someone tries to do
>>>> .withValue(x) in an out-of-range column
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>
>>>> Branch: refs/heads/CSV-216
>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>> Parents: 637ad2d
>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>> index 979119f..2be5c49 100644
>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>>>> Iterable<String> {
>>>>      public final CSVRecord immutable() {
>>>>         if (isMutable()) {
>>>>                 // Subclass is probably CSVMutableRecord, freeze values
>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>> values.length);
>>>> +               String[] frozenValue = values.clone();
>>>>                 return new CSVRecord(frozenValue, mapping, comment,
>>>> recordNumber, characterPosition);
>>>>         } else {
>>>>                 return this;
>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>>>> Iterable<String> {
>>>>         if (isMutable()) {
>>>>                 return this;
>>>>         }
>>>> -               String[] newValues = Arrays.copyOf(values,
>>>> values.length);
>>>> +               String[] newValues = values.clone();
>>>>         return new CSVMutableRecord(newValues, mapping, comment,
>>>> recordNumber, characterPosition);
>>>>         }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>



--
Stian Soiland-Reyes
http://orcid.org/0000-0001-9842-9718

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

sebb-2-2
On 16 February 2018 at 10:01, Stian Soiland-Reyes <[hidden email]> wrote:
> I agree in general for .clone() on objects - and I'm trying to move
> away from using .clone() in that Commons RDF fluent mutable/immutable
> builder (See COMMONSRDF-49).
>
> But this is an Object[] - a native type.

Huh?

The code says String[], which is native as it's final.

Object[] may not be native

> I must admit I am not sure what is really the preferred way - I
> thought .clone() is best as the VM can copy an array natively however
> it wants, while with Arrays.copyOf it might have to check if length is
> the same before doing anything clever.

In the specific case of String[] it might be OK to use clone, but in
general it's better to use the proper JVM methods and not try to
second-guess how efficient they are.

> On 13 February 2018 at 18:58, Gilles <[hidden email]> wrote:
>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>
>>> On 13 February 2018 at 09:31, Gilles <[hidden email]> wrote:
>>>>
>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>>>>
>>>>>
>>>>> Repository: commons-csv
>>>>> Updated Branches:
>>>>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>>
>>>>>
>>>>> CSV-216: Avoid Arrays.copyOf()
>>>>
>>>>
>>>>
>>>> Why?
>>>
>>>
>>> Agreed
>>>
>>>>> as .clone() will do
>>>>
>>>>
>>>>
>>>> We should rather avoid using "clone()".
>>>
>>>
>>> Again: why?
>>
>>
>> There are many discussions about this topic on the web.[1]
>> My point is that using "clone()" is not good advice.
>>
>> Gilles
>>
>> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>>
>>
>>> It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
>>> is considered bad.
>>>
>>>
>>>> Gilles
>>>>
>>>>
>>>>> -- at least until someone tries to do
>>>>> .withValue(x) in an out-of-range column
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>>> Commit:
>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>>
>>>>> Branch: refs/heads/CSV-216
>>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>>> Parents: 637ad2d
>>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>>
>>>>>
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>> index 979119f..2be5c49 100644
>>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>>>>> Iterable<String> {
>>>>>      public final CSVRecord immutable() {
>>>>>         if (isMutable()) {
>>>>>                 // Subclass is probably CSVMutableRecord, freeze values
>>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>>> values.length);
>>>>> +               String[] frozenValue = values.clone();
>>>>>                 return new CSVRecord(frozenValue, mapping, comment,
>>>>> recordNumber, characterPosition);
>>>>>         } else {
>>>>>                 return this;
>>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>>>>> Iterable<String> {
>>>>>         if (isMutable()) {
>>>>>                 return this;
>>>>>         }
>>>>> -               String[] newValues = Arrays.copyOf(values,
>>>>> values.length);
>>>>> +               String[] newValues = values.clone();
>>>>>         return new CSVMutableRecord(newValues, mapping, comment,
>>>>> recordNumber, characterPosition);
>>>>>         }
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>
>
>
> --
> Stian Soiland-Reyes
> http://orcid.org/0000-0001-9842-9718
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Stian Soiland-Reyes
String is still a type of Object (requiring GC handling of reference
counters), as you can do Object[] o = stringArray.clone(); without
casting.  (other way not so)

https://www.javaspecialists.eu/archive/Issue124.html says there is
hardly any difference, so I don't think we need to fight this one out
and can just go with the cleanest code :)

On 16 February 2018 at 11:22, sebb <[hidden email]> wrote:

> On 16 February 2018 at 10:01, Stian Soiland-Reyes <[hidden email]> wrote:
>> I agree in general for .clone() on objects - and I'm trying to move
>> away from using .clone() in that Commons RDF fluent mutable/immutable
>> builder (See COMMONSRDF-49).
>>
>> But this is an Object[] - a native type.
>
> Huh?
>
> The code says String[], which is native as it's final.
>
> Object[] may not be native
>
>> I must admit I am not sure what is really the preferred way - I
>> thought .clone() is best as the VM can copy an array natively however
>> it wants, while with Arrays.copyOf it might have to check if length is
>> the same before doing anything clever.
>
> In the specific case of String[] it might be OK to use clone, but in
> general it's better to use the proper JVM methods and not try to
> second-guess how efficient they are.
>
>> On 13 February 2018 at 18:58, Gilles <[hidden email]> wrote:
>>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>>
>>>> On 13 February 2018 at 09:31, Gilles <[hidden email]> wrote:
>>>>>
>>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>>>>>
>>>>>>
>>>>>> Repository: commons-csv
>>>>>> Updated Branches:
>>>>>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>>>
>>>>>>
>>>>>> CSV-216: Avoid Arrays.copyOf()
>>>>>
>>>>>
>>>>>
>>>>> Why?
>>>>
>>>>
>>>> Agreed
>>>>
>>>>>> as .clone() will do
>>>>>
>>>>>
>>>>>
>>>>> We should rather avoid using "clone()".
>>>>
>>>>
>>>> Again: why?
>>>
>>>
>>> There are many discussions about this topic on the web.[1]
>>> My point is that using "clone()" is not good advice.
>>>
>>> Gilles
>>>
>>> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>>>
>>>
>>>> It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
>>>> is considered bad.
>>>>
>>>>
>>>>> Gilles
>>>>>
>>>>>
>>>>>> -- at least until someone tries to do
>>>>>> .withValue(x) in an out-of-range column
>>>>>>
>>>>>>
>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>>>> Commit:
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>>>
>>>>>> Branch: refs/heads/CSV-216
>>>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>>>> Parents: 637ad2d
>>>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>>>
>>>>>>
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>> index 979119f..2be5c49 100644
>>>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>>>>>> Iterable<String> {
>>>>>>      public final CSVRecord immutable() {
>>>>>>         if (isMutable()) {
>>>>>>                 // Subclass is probably CSVMutableRecord, freeze values
>>>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>>>> values.length);
>>>>>> +               String[] frozenValue = values.clone();
>>>>>>                 return new CSVRecord(frozenValue, mapping, comment,
>>>>>> recordNumber, characterPosition);
>>>>>>         } else {
>>>>>>                 return this;
>>>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>>>>>> Iterable<String> {
>>>>>>         if (isMutable()) {
>>>>>>                 return this;
>>>>>>         }
>>>>>> -               String[] newValues = Arrays.copyOf(values,
>>>>>> values.length);
>>>>>> +               String[] newValues = values.clone();
>>>>>>         return new CSVMutableRecord(newValues, mapping, comment,
>>>>>> recordNumber, characterPosition);
>>>>>>         }
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>
>>
>>
>> --
>> Stian Soiland-Reyes
>> http://orcid.org/0000-0001-9842-9718
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>



--
Stian Soiland-Reyes
http://orcid.org/0000-0001-9842-9718

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Gilles Sadowski
On Fri, 16 Feb 2018 13:53:28 +0000, Stian Soiland-Reyes wrote:
> String is still a type of Object (requiring GC handling of reference
> counters), as you can do Object[] o = stringArray.clone(); without
> casting.  (other way not so)
>
> https://www.javaspecialists.eu/archive/Issue124.html says there is
> hardly any difference, so I don't think we need to fight this one out
> and can just go with the cleanest code :)

+1

That was my point.
Before "Arrays" (Java 1.6), I also used to "clone" arrays, in
preference to "System.arraycopy" ;-)

Gilles

>
> On 16 February 2018 at 11:22, sebb <[hidden email]> wrote:
>> On 16 February 2018 at 10:01, Stian Soiland-Reyes <[hidden email]>
>> wrote:
>>> I agree in general for .clone() on objects - and I'm trying to move
>>> away from using .clone() in that Commons RDF fluent
>>> mutable/immutable
>>> builder (See COMMONSRDF-49).
>>>
>>> But this is an Object[] - a native type.
>>
>> Huh?
>>
>> The code says String[], which is native as it's final.
>>
>> Object[] may not be native
>>
>>> I must admit I am not sure what is really the preferred way - I
>>> thought .clone() is best as the VM can copy an array natively
>>> however
>>> it wants, while with Arrays.copyOf it might have to check if length
>>> is
>>> the same before doing anything clever.
>>
>> In the specific case of String[] it might be OK to use clone, but in
>> general it's better to use the proper JVM methods and not try to
>> second-guess how efficient they are.
>>
>>> On 13 February 2018 at 18:58, Gilles <[hidden email]>
>>> wrote:
>>>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>>>
>>>>> On 13 February 2018 at 09:31, Gilles
>>>>> <[hidden email]> wrote:
>>>>>>
>>>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email]
>>>>>> wrote:
>>>>>>>
>>>>>>>
>>>>>>> Repository: commons-csv
>>>>>>> Updated Branches:
>>>>>>>   refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>>>>
>>>>>>>
>>>>>>> CSV-216: Avoid Arrays.copyOf()
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why?
>>>>>
>>>>>
>>>>> Agreed
>>>>>
>>>>>>> as .clone() will do
>>>>>>
>>>>>>
>>>>>>
>>>>>> We should rather avoid using "clone()".
>>>>>
>>>>>
>>>>> Again: why?
>>>>
>>>>
>>>> There are many discussions about this topic on the web.[1]
>>>> My point is that using "clone()" is not good advice.
>>>>
>>>> Gilles
>>>>
>>>> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>>>>
>>>>
>>>>> It's not clear why Arrays.copyOf(0 is considered bad, nor why
>>>>> clone()
>>>>> is considered bad.
>>>>>
>>>>>
>>>>>> Gilles
>>>>>>
>>>>>>
>>>>>>> -- at least until someone tries to do
>>>>>>> .withValue(x) in an out-of-range column
>>>>>>>
>>>>>>>
>>>>>>> Project:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>>>>> Commit:
>>>>>>>
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>>>>> Tree:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>>>>> Diff:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>>>>
>>>>>>> Branch: refs/heads/CSV-216
>>>>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>>>>> Parents: 637ad2d
>>>>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>>  src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> diff --git
>>>>>>> a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> index 979119f..2be5c49 100644
>>>>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements
>>>>>>> Serializable,
>>>>>>> Iterable<String> {
>>>>>>>      public final CSVRecord immutable() {
>>>>>>>         if (isMutable()) {
>>>>>>>                 // Subclass is probably CSVMutableRecord,
>>>>>>> freeze values
>>>>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>>>>> values.length);
>>>>>>> +               String[] frozenValue = values.clone();
>>>>>>>                 return new CSVRecord(frozenValue, mapping,
>>>>>>> comment,
>>>>>>> recordNumber, characterPosition);
>>>>>>>         } else {
>>>>>>>                 return this;
>>>>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements
>>>>>>> Serializable,
>>>>>>> Iterable<String> {
>>>>>>>         if (isMutable()) {
>>>>>>>                 return this;
>>>>>>>         }
>>>>>>> -               String[] newValues = Arrays.copyOf(values,
>>>>>>> values.length);
>>>>>>> +               String[] newValues = values.clone();
>>>>>>>         return new CSVMutableRecord(newValues, mapping,
>>>>>>> comment,
>>>>>>> recordNumber, characterPosition);
>>>>>>>         }
>>>>
>>>>
>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Charles Honton
In reply to this post by Stian Soiland-Reyes
Read Bloch.  Don’t optimize until you have proven that this bit of code is causing a significant performance hit.
arrayCopy can and is inlined by jit.

Chas

> On Feb 16, 2018, at 5:53 AM, Stian Soiland-Reyes <[hidden email]> wrote:
>
> String is still a type of Object (requiring GC handling of reference
> counters), as you can do Object[] o = stringArray.clone(); without
> casting.  (other way not so)
>
> https://www.javaspecialists.eu/archive/Issue124.html says there is
> hardly any difference, so I don't think we need to fight this one out
> and can just go with the cleanest code :)
>
>> On 16 February 2018 at 11:22, sebb <[hidden email]> wrote:
>>> On 16 February 2018 at 10:01, Stian Soiland-Reyes <[hidden email]> wrote:
>>> I agree in general for .clone() on objects - and I'm trying to move
>>> away from using .clone() in that Commons RDF fluent mutable/immutable
>>> builder (See COMMONSRDF-49).
>>>
>>> But this is an Object[] - a native type.
>>
>> Huh?
>>
>> The code says String[], which is native as it's final.
>>
>> Object[] may not be native
>>
>>> I must admit I am not sure what is really the preferred way - I
>>> thought .clone() is best as the VM can copy an array natively however
>>> it wants, while with Arrays.copyOf it might have to check if length is
>>> the same before doing anything clever.
>>
>> In the specific case of String[] it might be OK to use clone, but in
>> general it's better to use the proper JVM methods and not try to
>> second-guess how efficient they are.
>>
>>>> On 13 February 2018 at 18:58, Gilles <[hidden email]> wrote:
>>>>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>>>
>>>>>> On 13 February 2018 at 09:31, Gilles <[hidden email]> wrote:
>>>>>>
>>>>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email] wrote:
>>>>>>>
>>>>>>>
>>>>>>> Repository: commons-csv
>>>>>>> Updated Branches:
>>>>>>>  refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>>>>
>>>>>>>
>>>>>>> CSV-216: Avoid Arrays.copyOf()
>>>>>>
>>>>>>
>>>>>>
>>>>>> Why?
>>>>>
>>>>>
>>>>> Agreed
>>>>>
>>>>>>> as .clone() will do
>>>>>>
>>>>>>
>>>>>>
>>>>>> We should rather avoid using "clone()".
>>>>>
>>>>>
>>>>> Again: why?
>>>>
>>>>
>>>> There are many discussions about this topic on the web.[1]
>>>> My point is that using "clone()" is not good advice.
>>>>
>>>> Gilles
>>>>
>>>> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>>>>
>>>>
>>>>> It's not clear why Arrays.copyOf(0 is considered bad, nor why clone()
>>>>> is considered bad.
>>>>>
>>>>>
>>>>>> Gilles
>>>>>>
>>>>>>
>>>>>>> -- at least until someone tries to do
>>>>>>> .withValue(x) in an out-of-range column
>>>>>>>
>>>>>>>
>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>>>>> Commit:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>>>>
>>>>>>> Branch: refs/heads/CSV-216
>>>>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>>>>> Parents: 637ad2d
>>>>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> diff --git a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> index 979119f..2be5c49 100644
>>>>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements Serializable,
>>>>>>> Iterable<String> {
>>>>>>>     public final CSVRecord immutable() {
>>>>>>>        if (isMutable()) {
>>>>>>>                // Subclass is probably CSVMutableRecord, freeze values
>>>>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>>>>> values.length);
>>>>>>> +               String[] frozenValue = values.clone();
>>>>>>>                return new CSVRecord(frozenValue, mapping, comment,
>>>>>>> recordNumber, characterPosition);
>>>>>>>        } else {
>>>>>>>                return this;
>>>>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements Serializable,
>>>>>>> Iterable<String> {
>>>>>>>        if (isMutable()) {
>>>>>>>                return this;
>>>>>>>        }
>>>>>>> -               String[] newValues = Arrays.copyOf(values,
>>>>>>> values.length);
>>>>>>> +               String[] newValues = values.clone();
>>>>>>>        return new CSVMutableRecord(newValues, mapping, comment,
>>>>>>> recordNumber, characterPosition);
>>>>>>>        }
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [hidden email]
>>>> For additional commands, e-mail: [hidden email]
>>>>
>>>
>>>
>>>
>>> --
>>> Stian Soiland-Reyes
>>> http://orcid.org/0000-0001-9842-9718
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>
>
>
> --
> Stian Soiland-Reyes
> http://orcid.org/0000-0001-9842-9718
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Gilles Sadowski
On Fri, 16 Feb 2018 08:33:46 -0800, Chas Honton wrote:
> Read Bloch.  Don’t optimize until you have proven that this bit of
> code is causing a significant performance hit.
> arrayCopy can and is inlined by jit

https://www.artima.com/intv/bloch13.html

Bottom-line: Don't use clone except to copy arrays.
So I'd rephrase my comment on the commit: let "clone()" in place
but with the comment that it's the only acceptable use of it.

Regards,
Gilles

>
> Chas
>
>> On Feb 16, 2018, at 5:53 AM, Stian Soiland-Reyes <[hidden email]>
>> wrote:
>>
>> String is still a type of Object (requiring GC handling of reference
>> counters), as you can do Object[] o = stringArray.clone(); without
>> casting.  (other way not so)
>>
>> https://www.javaspecialists.eu/archive/Issue124.html says there is
>> hardly any difference, so I don't think we need to fight this one
>> out
>> and can just go with the cleanest code :)
>>
>>> On 16 February 2018 at 11:22, sebb <[hidden email]> wrote:
>>>> On 16 February 2018 at 10:01, Stian Soiland-Reyes
>>>> <[hidden email]> wrote:
>>>> I agree in general for .clone() on objects - and I'm trying to
>>>> move
>>>> away from using .clone() in that Commons RDF fluent
>>>> mutable/immutable
>>>> builder (See COMMONSRDF-49).
>>>>
>>>> But this is an Object[] - a native type.
>>>
>>> Huh?
>>>
>>> The code says String[], which is native as it's final.
>>>
>>> Object[] may not be native
>>>
>>>> I must admit I am not sure what is really the preferred way - I
>>>> thought .clone() is best as the VM can copy an array natively
>>>> however
>>>> it wants, while with Arrays.copyOf it might have to check if
>>>> length is
>>>> the same before doing anything clever.
>>>
>>> In the specific case of String[] it might be OK to use clone, but
>>> in
>>> general it's better to use the proper JVM methods and not try to
>>> second-guess how efficient they are.
>>>
>>>>> On 13 February 2018 at 18:58, Gilles
>>>>> <[hidden email]> wrote:
>>>>>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>>>>
>>>>>>> On 13 February 2018 at 09:31, Gilles
>>>>>>> <[hidden email]> wrote:
>>>>>>>
>>>>>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), [hidden email]
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Repository: commons-csv
>>>>>>>> Updated Branches:
>>>>>>>>  refs/heads/CSV-216 637ad2d7a -> f66a83901
>>>>>>>>
>>>>>>>>
>>>>>>>> CSV-216: Avoid Arrays.copyOf()
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Why?
>>>>>>
>>>>>>
>>>>>> Agreed
>>>>>>
>>>>>>>> as .clone() will do
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> We should rather avoid using "clone()".
>>>>>>
>>>>>>
>>>>>> Again: why?
>>>>>
>>>>>
>>>>> There are many discussions about this topic on the web.[1]
>>>>> My point is that using "clone()" is not good advice.
>>>>>
>>>>> Gilles
>>>>>
>>>>> [1] E.g. http://vojtechruzicka.com/java-cloning-problems/
>>>>>
>>>>>
>>>>>> It's not clear why Arrays.copyOf(0 is considered bad, nor why
>>>>>> clone()
>>>>>> is considered bad.
>>>>>>
>>>>>>
>>>>>>> Gilles
>>>>>>>
>>>>>>>
>>>>>>>> -- at least until someone tries to do
>>>>>>>> .withValue(x) in an out-of-range column
>>>>>>>>
>>>>>>>>
>>>>>>>> Project:
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/repo
>>>>>>>> Commit:
>>>>>>>>
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/commit/f66a8390
>>>>>>>> Tree:
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/tree/f66a8390
>>>>>>>> Diff:
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/diff/f66a8390
>>>>>>>>
>>>>>>>> Branch: refs/heads/CSV-216
>>>>>>>> Commit: f66a83901bd026369a2e8d522bd567eb2ef3f8c0
>>>>>>>> Parents: 637ad2d
>>>>>>>> Author: Stian Soiland-Reyes <[hidden email]>
>>>>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>>>>> Committer: Stian Soiland-Reyes <[hidden email]>
>>>>>>>> Committed: Tue Feb 13 00:14:52 2018 +0000
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>> src/main/java/org/apache/commons/csv/CSVRecord.java | 4 ++--
>>>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-csv/blob/f66a8390/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>> diff --git
>>>>>>>> a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>> b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>> index 979119f..2be5c49 100644
>>>>>>>> --- a/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>> +++ b/src/main/java/org/apache/commons/csv/CSVRecord.java
>>>>>>>> @@ -199,7 +199,7 @@ public class CSVRecord implements
>>>>>>>> Serializable,
>>>>>>>> Iterable<String> {
>>>>>>>>     public final CSVRecord immutable() {
>>>>>>>>        if (isMutable()) {
>>>>>>>>                // Subclass is probably CSVMutableRecord,
>>>>>>>> freeze values
>>>>>>>> -               String[] frozenValue = Arrays.copyOf(values,
>>>>>>>> values.length);
>>>>>>>> +               String[] frozenValue = values.clone();
>>>>>>>>                return new CSVRecord(frozenValue, mapping,
>>>>>>>> comment,
>>>>>>>> recordNumber, characterPosition);
>>>>>>>>        } else {
>>>>>>>>                return this;
>>>>>>>> @@ -260,7 +260,7 @@ public class CSVRecord implements
>>>>>>>> Serializable,
>>>>>>>> Iterable<String> {
>>>>>>>>        if (isMutable()) {
>>>>>>>>                return this;
>>>>>>>>        }
>>>>>>>> -               String[] newValues = Arrays.copyOf(values,
>>>>>>>> values.length);
>>>>>>>> +               String[] newValues = values.clone();
>>>>>>>>        return new CSVMutableRecord(newValues, mapping,
>>>>>>>> comment,
>>>>>>>> recordNumber, characterPosition);
>>>>>>>>        }


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()

Stian Soiland-Reyes
On Fri, 16 Feb 2018 20:11:11 +0100, Gilles <[hidden email]> wrote:
> Bottom-line: Don't use clone except to copy arrays.
> So I'd rephrase my comment on the commit: let "clone()" in place
> but with the comment that it's the only acceptable use of it.

OK, I've added such comments :)

(It was also used for copying the headers, which was also String[])

--
Stian Soiland-Reyes
The University of Manchester
http://www.esciencelab.org.uk/
http://orcid.org/0000-0001-9842-9718


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]