Skip to content

Add StringBuilder.AppendJoin#21

Merged
neuecc merged 4 commits intoCysharp:masterfrom
udaken:add-StringBuilder-AppendJoin
Aug 3, 2020
Merged

Add StringBuilder.AppendJoin#21
neuecc merged 4 commits intoCysharp:masterfrom
udaken:add-StringBuilder-AppendJoin

Conversation

@udaken
Copy link
Copy Markdown
Contributor

@udaken udaken commented Jul 4, 2020

There is no way to join the current ZString to avoid the allocation.
The BCL includes a StringBuilder.AppendJoin Method.
https://docs.microsoft.com/en-us/dotnet/api/system.text.stringbuilder.appendjoin?view=netcore-3.1

I have implemented AppendJoin to ZString.

NOTE: This PR is waiting for #19 and #20.

@udaken udaken force-pushed the add-StringBuilder-AppendJoin branch from ccf417c to 6a301ba Compare July 30, 2020 13:20
@udaken udaken marked this pull request as ready for review July 30, 2020 13:20
@udaken
Copy link
Copy Markdown
Contributor Author

udaken commented Jul 30, 2020

The dependent PRs have been merged and the branch has been updated.

@udaken
Copy link
Copy Markdown
Contributor Author

udaken commented Jul 30, 2020

For testing, I couldn't find a class that implements IList<T> and doesn't implement IReadOnlyList<T> in BCL.

Therefore, I apologize for not testing well against the ReadOnlyListAdaptor<T>.

@neuecc
Copy link
Copy Markdown
Member

neuecc commented Jul 31, 2020

Thanks for the many contributes.
I checked this and #24, and ok to merge.
If merged it, I'll release new version so could you resolve conflict #21 and #24?

@udaken
Copy link
Copy Markdown
Contributor Author

udaken commented Jul 31, 2020

@neuecc Thanks for the review.
I fixed the conflict with #22.

@neuecc neuecc merged commit 01ff05c into Cysharp:master Aug 3, 2020
@udaken udaken deleted the add-StringBuilder-AppendJoin branch August 3, 2020 15:08
ThangwLee pushed a commit to WolffunGame/ZString that referenced this pull request May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants