Skip to content

Implement ZString.Concat works as same as string.Concat on collection types#19

Merged
neuecc merged 3 commits intoCysharp:masterfrom
piti6:implement-Concat-from-collections
Jul 30, 2020
Merged

Implement ZString.Concat works as same as string.Concat on collection types#19
neuecc merged 3 commits intoCysharp:masterfrom
piti6:implement-Concat-from-collections

Conversation

@piti6
Copy link
Copy Markdown
Contributor

@piti6 piti6 commented Jun 29, 2020

Using of ZString.Concat on collection types leads to the wrong result that users would expecting the result as same as string.Concat.

https://github.com/Cysharp/ZString/compare/master...piti6:implement-Concat-from-collections?expand=1#diff-eb70c70af0fdc6be03f85a109ae3909dR136-R145

Expected: Test succeeds without failure.
Result:

Expected ZString.Concat(values) to be 
    "abcdef" with a length of 6, but 
    "System.String[]" has a length of 15, differs near "Sys" (index 0).

@udaken
Copy link
Copy Markdown
Contributor

udaken commented Jul 2, 2020

Hi @piti6 .

What about using JoinInternal instead of implementing ConcatInternal to reduce code?
It is like this.

        public static string Concat<T>(params T[] values)
        {
            return JoinInternal<T>(default, values.AsSpan());
        }

@piti6
Copy link
Copy Markdown
Contributor Author

piti6 commented Jul 2, 2020

@udaken I`m pretty sure they are almost same and as I used your code as reference, it would be good choice. I'll fix it. Thank you!

@neuecc
Copy link
Copy Markdown
Member

neuecc commented Jul 30, 2020

Sorry for the delayed response.
Thanks, good improvment.

@neuecc neuecc merged commit 095c6d7 into Cysharp:master Jul 30, 2020
@piti6 piti6 deleted the implement-Concat-from-collections branch July 30, 2020 13:23
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.

3 participants