-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce String.join
function
#2762
Conversation
92e5bfb
to
0674fa6
Compare
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #2762 +/- ##
==========================================
+ Coverage 79.18% 79.22% +0.04%
==========================================
Files 333 333
Lines 78614 78719 +105
==========================================
+ Hits 62248 62365 +117
+ Misses 14063 14045 -18
- Partials 2303 2309 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@SupunS Contributors can't add labels themselves, please add one for the release notes |
…parator, add test case
…ual creation of string memory usage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Regarding the default: Every language's standard library seems to have a different default, so maybe choosing the empty string as the "lowest common denominator" is best, as it avoids surprises based on wrong expectations.
Maybe we should just require it, i.e. not have any default?
For example:
- Swift:
""
(empty string) - JavaScript:
","
- Kotlin:
", "
(additional space) - Python, Rust, Go: no default
I think it is probably best to just make it required. Made it required in b92d5e1. If we want to have a default, then I would prefer having the same default in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this function doesn't mutate the input array, so making a note to myself to add the appropriate view
annotation as per #2466
@SupunS @turbolent Should we merge this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
Co-authored-by: Bastian Müller <[email protected]>
Work towards #2752
Description
Adds
join(_ strs: [String], _ separator: String): String
function to Cadence. The function returns a string value after concatenating elements of the provided array of string with the given separator.master
branchFiles changed
in the Github PR explorer