Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Use _emptyArray in List<T>.ToArray #2758

Merged
merged 1 commit into from
Jan 21, 2016
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion src/mscorlib/src/System/Collections/Generic/List.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1001,12 +1001,19 @@ public void Sort(Comparison<T> comparison) {
}
}

// ToArray returns a new Object array containing the contents of the List.
// ToArray returns an array containing the contents of the List.
// This requires copying the List, which is an O(n) operation.
public T[] ToArray() {
Contract.Ensures(Contract.Result<T[]>() != null);
Contract.Ensures(Contract.Result<T[]>().Length == Count);

#if FEATURE_CORECLR
if (_size == 0)
{
return _emptyArray;
}
#endif

T[] array = new T[_size];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you consider adding a method like the following:

T[] Allocate<T>(int size) 
{
#if FEATURE_CORCLR
  if (size == 0) { return Array.Empty<T>(); }
#endif
  return new T[size];
}

This could then become a drop in replacement for new T[size] in places where Array.Empty<T> was acceptable.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea. Though in a case like this (and I expect others) you'd end up doing something like:

T[] array = Allocate(_size);
Array.Copy(_items, 0, array, 0, _size);
return array;

and you'd be doing a bit of extra work unnecessarily with the Array.Copy call.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree there would be some extra work here in copy. Feel like more Array.Empty<T> overall would get more use if we have a clean, simple way to have allocations return it automatically.

Array.Copy(_items, 0, array, 0, _size);
return array;
Expand Down