commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: commons-csv git commit: CSV-216: Avoid Arrays.copyOf()
Date Fri, 16 Feb 2018 16:31:16 GMT
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 <sebbaz@gmail.com> wrote:
>> 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


Mime
View raw message