commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()
Date Fri, 16 Feb 2018 11:22:26 GMT
On 16 February 2018 at 10:01, Stian Soiland-Reyes <stain@apache.org> 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 <gilles@harfang.homelinux.org> wrote:
>> On Tue, 13 Feb 2018 18:40:13 +0000, sebb wrote:
>>>
>>> On 13 February 2018 at 09:31, Gilles <gilles@harfang.homelinux.org> wrote:
>>>>
>>>> On Tue, 13 Feb 2018 00:16:13 +0000 (UTC), stain@apache.org 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 <stain@apache.org>
>>>>> Authored: Fri Feb 9 16:49:51 2018 +0000
>>>>> Committer: Stian Soiland-Reyes <stain@apache.org>
>>>>> 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: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
>
> --
> Stian Soiland-Reyes
> http://orcid.org/0000-0001-9842-9718
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

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


Mime
View raw message