Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
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
5 changes: 5 additions & 0 deletions src/mscorlib/model.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7212,6 +7212,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
195 changes: 191 additions & 4 deletions src/mscorlib/src/System/String.Manipulation.cs
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.
//
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: Remove dead //s

public static String Join(Char separator, params String[] value) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: Use string, char, and string.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Also put the brace on a newline.

if (value == null)
throw new ArgumentNullException("value");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: put braces here.


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.
//
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: dead // (same in other places)

[ComVisible(false)]
public static string Join(string separator, params object[] values)
{
Expand Down Expand Up @@ -584,6 +595,42 @@ public static string Join(string separator, params object[] values)
return StringBuilderCache.GetStringAndRelease(result);
}

// Joins an array of objects together as one string with a char separator between each original string.
//
[ComVisible(false)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are the ComVisibles still necessary?

public static String Join(Char separator, params Object[] values) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: string, char, object. (same in other places)

if (values == null)
throw new ArgumentNullException("values");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: add braces. (same in other places)


if (values.Length == 0 || values[0] == null)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We don't want to propogate the bug where string.Join(string, object[]) returns null if the first item is null (even if the object has some non-null items) to more places.

return String.Empty;
Contract.EndContractBlock();

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 +682,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())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: Spacing

{
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 +752,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
Copy Markdown
Author

Choose a reason for hiding this comment

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

@terrajobst @weshaggard @stephentoub - Do we still want to keep this overload?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why wouldn't we keep it?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh I see @stephentoub comment on the old PR. I agree with @stephentoub that it isn't really needed unless we see some sort of other benefit.

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

using(IEnumerator<String> en = values.GetEnumerator()) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nit: spacing, String.

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 +856,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;
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
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 ) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit why the space after ( and before )?

if ( (m_totalSize - m_length) < 1 ) {
throw new IndexOutOfRangeException();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should probably look into changing this method (probably delete the class entirely) afterwards. In its current state this does not look like it will be inlined.

}

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