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

Adds String.Join overload to accept char separator #2945

Closed
wants to merge 1 commit into from
Closed

Adds String.Join overload to accept char separator #2945

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions src/mscorlib/model.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7096,6 +7096,11 @@
<Member Name="Join(System.String,System.Collections.Generic.IEnumerable&lt;System.String&gt;)" />
<Member Name="Join(System.String,System.String[])" />
<Member Name="Join(System.String,System.String[],System.Int32,System.Int32)" />
<Member Name="Join(System.Char,System.Object[])" />
<Member Name="Join&lt;T&gt;(System.Char,System.Collections.Generic.IEnumerable&lt;T&gt;)" />
<Member Name="Join(System.Char,System.Collections.Generic.IEnumerable&lt;System.String&gt;)" />
<Member Name="Join(System.Char,System.String[])" />
<Member Name="Join(System.Char,System.String[],System.Int32,System.Int32)" />
<Member Name="LastIndexOf(System.Char)" />
<Member Name="LastIndexOf(System.Char,System.Int32)" />
<Member Name="LastIndexOf(System.Char,System.Int32,System.Int32)" />
Expand Down
194 changes: 190 additions & 4 deletions src/mscorlib/src/System/String.Manipulation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -541,16 +541,27 @@ public String Insert(int startIndex, String value)
}
return result;
}
// Joins an array of strings together as one string with a separator between each original string.

// Joins an array of strings together as one string with a string separator between each original string.
//
public static String Join(String separator, params String[] value) {
if (value==null)
if (value == null)
throw new ArgumentNullException("value");
Contract.EndContractBlock();
return Join(separator, value, 0, value.Length);
}

// Joins an array of strings together as one string with a char separator between each original string.
//
public static String Join(Char separator, params String[] value) {
if (value == null)
throw new ArgumentNullException("value");

return Join(separator, value, 0, value.Length);
}

// Joins an object array of strings together as one string with a string separator between each original string.
//
[ComVisible(false)]
public static string Join(string separator, params object[] values)
{
Expand Down Expand Up @@ -584,6 +595,41 @@ public static string Join(string separator, params object[] values)
return StringBuilderCache.GetStringAndRelease(result);
}

// Joins an object array of strings together as one string with a char separator between each original string.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: comment is incorrect; this overload takes an array of objects, not strings

//
[ComVisible(false)]
public static String Join(Char separator, params Object[] values) {
if (values == null)
throw new ArgumentNullException("values");

Copy link
Member

Choose a reason for hiding this comment

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

Nit: Contract.EndContractBlock wasn't added here but it was added in the overload below. We don't use the Contract stuff anymore, but we should be consistent.

if (values.Length == 0 || values[0] == null)
return String.Empty;

string firstString = values[0].ToString();

if (values.Length == 1)
{
return firstString ?? string.Empty;
}

StringBuilder result = StringBuilderCache.Acquire();
result.Append(firstString);

for (int i = 1; i < values.Length; i++)
{
result.Append(separator);
object value = values[i];
if (value != null)
{
result.Append(value.ToString());
}
}

return StringBuilderCache.GetStringAndRelease(result);
}

// Joins a generic IEnumerable together as one string with a string separator between each original string.
//
[ComVisible(false)]
public static String Join<T>(String separator, IEnumerable<T> values)
{
Expand Down Expand Up @@ -635,6 +681,46 @@ public static String Join<T>(String separator, IEnumerable<T> values)
}
}

// Joins a generic IEnumerable together as one string with a char separator between each original string.
//
[ComVisible(false)]
public static String Join<T>(Char separator, IEnumerable<T> values)
{
if (values == null)
throw new ArgumentNullException("values");
Contract.Ensures(Contract.Result<String>() != null);
Contract.EndContractBlock();

using(IEnumerator<T> en = values.GetEnumerator())
{
if (!en.MoveNext())
return String.Empty;

StringBuilder result = StringBuilderCache.Acquire();
T currentValue = en.Current;

if (currentValue != null)
{
result.Append(currentValue.ToString());
}

while (en.MoveNext())
{
result.Append(separator);
currentValue = en.Current;

if (currentValue != null)
{
result.Append(currentValue.ToString());
}
}

return StringBuilderCache.GetStringAndRelease(result);
}
}

// Joins a string IEnumerable together as one string with a string separator between each original string.
//
[ComVisible(false)]
public static String Join(String separator, IEnumerable<String> values) {
if (values == null)
Expand Down Expand Up @@ -665,7 +751,39 @@ public static String Join(String separator, IEnumerable<String> values) {
}
}

// Joins an array of strings together as one string with a separator between each original string.
// Joins a string IEnumerable together as one string with a char separator between each original string.
//
[ComVisible(false)]
public static String Join(Char separator, IEnumerable<String> values) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this overload? The use case is covered entirely by the Join(char, IEnumerable<T>) overload, and the only benefit I see to this one is avoiding per-element a branch and a ToString call (which will just return this). Is the performance difference between, say, string.Join('a', Enumerable.Repeat("b", 10000000)) and string.Join<string>('a', Enumerable.Repeat("b", 10000000)) different enough that it's worth having the extra overload?

Copy link

Choose a reason for hiding this comment

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

@terrajobst @weshaggard as you guys reviewed these APIs do you have any second thoughts about this?

if (values == null)
throw new ArgumentNullException("values");
Contract.Ensures(Contract.Result<String>() != null);
Contract.EndContractBlock();

using(IEnumerator<String> en = values.GetEnumerator()) {
if (!en.MoveNext())
return String.Empty;

String firstValue = en.Current;

if (!en.MoveNext()) {
// Only one value available
return firstValue ?? String.Empty;
}

// Null separator and values are handled by the StringBuilder
StringBuilder result = StringBuilderCache.Acquire();
result.Append(firstValue);

do {
result.Append(separator);
result.Append(en.Current);
} while (en.MoveNext());
return StringBuilderCache.GetStringAndRelease(result);
}
}

// Joins an array of strings together as one string with a string separator between each original string.
//
[System.Security.SecuritySafeCritical] // auto-generated
public unsafe static String Join(String separator, String[] value, int startIndex, int count) {
Expand Down Expand Up @@ -737,6 +855,74 @@ public unsafe static String Join(String separator, String[] value, int startInde

return jointString;
}

// Joins an array of strings together as one string with a char separator between each original string.
//
[System.Security.SecuritySafeCritical] // auto-generated
public unsafe static String Join(Char separator, String[] value, int startIndex, int count) {
//Range check the array
if (value == null)
throw new ArgumentNullException("value");

if (startIndex < 0)
throw new ArgumentOutOfRangeException("startIndex", Environment.GetResourceString("ArgumentOutOfRange_StartIndex"));
if (count < 0)
throw new ArgumentOutOfRangeException("count", Environment.GetResourceString("ArgumentOutOfRange_NegativeCount"));

if (startIndex > value.Length - count)
throw new ArgumentOutOfRangeException("startIndex", Environment.GetResourceString("ArgumentOutOfRange_IndexCountBuffer"));
Contract.EndContractBlock();

//If count is 0, that skews a whole bunch of the calculations below, so just special case that.
if (count == 0) {
return String.Empty;
}

if (count == 1) {
return value[startIndex] ?? String.Empty;
}

int jointLength = 0;
//Figure out the total length of the strings in value
int endIndex = startIndex + count - 1;
for (int stringToJoinIndex = startIndex; stringToJoinIndex <= endIndex; stringToJoinIndex++) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not:

int endIndex = startIndex + count;
for (int stringToJoinIndex = startIndex; stringToJoinIndex < endIndex; stringToJoinIndex++) {

?

string currentValue = value[stringToJoinIndex];

if (currentValue != null) {
jointLength += currentValue.Length;
}
}

//Add enough room for the separator.
jointLength += count - 1;

// Note that we may not catch all overflows with this check (since we could have wrapped around the 4gb range any number of times
// and landed back in the positive range.) The input array might be modifed from other threads,
// so we have to do an overflow check before each append below anyway. Those overflows will get caught down there.
if ((jointLength < 0) || ((jointLength + 1) < 0) ) {
throw new OutOfMemoryException();
}

//If this is an empty string, just return.
if (jointLength == 0) {
return String.Empty;
}

string jointString = FastAllocateString( jointLength );
fixed (char * pointerToJointString = &jointString.m_firstChar) {
UnSafeCharBuffer charBuffer = new UnSafeCharBuffer( pointerToJointString, jointLength);

// Append the first string first and then append each following string prefixed by the separator.
charBuffer.AppendString( value[startIndex] );
for (int stringToJoinIndex = startIndex + 1; stringToJoinIndex <= endIndex; stringToJoinIndex++) {
charBuffer.AppendChar( separator );
charBuffer.AppendString( value[stringToJoinIndex] );
}
Contract.Assert(*(pointerToJointString + charBuffer.Length) == '\0', "String must be null-terminated!");
}

return jointString;
}

//
//
Expand Down
25 changes: 18 additions & 7 deletions src/mscorlib/src/System/UnSafeCharBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,36 @@ public UnSafeCharBuffer( char *buffer, int bufferSize) {
m_totalSize = bufferSize;
m_length = 0;
}


[System.Security.SecuritySafeCritical] // auto-generated
public void AppendChar( char charToAppend ) {
if ( (m_totalSize - m_length) < 1 ) {
throw new IndexOutOfRangeException();
}

m_buffer[m_length++] = charToAppend;

Contract.Assert(m_length <= m_totalSize, "Buffer has been overflowed!");
}

[System.Security.SecuritySafeCritical] // auto-generated
public void AppendString(string stringToAppend) {
if( String.IsNullOrEmpty( stringToAppend ) ) {
return;
}

if ( (m_totalSize - m_length) < stringToAppend.Length ) {
throw new IndexOutOfRangeException();
}
fixed( char* pointerToString = stringToAppend ) {

fixed( char* pointerToString = stringToAppend ) {
Buffer.Memcpy( (byte*) (m_buffer + m_length), (byte *) pointerToString, stringToAppend.Length * sizeof(char));
}
}

m_length += stringToAppend.Length;
Contract.Assert(m_length <= m_totalSize, "Buffer has been overflowed!");
}

public int Length {
get {
return m_length;
Expand Down