Skip to content

add binary:join/2#427

Closed
trollfred wants to merge 1 commit intoerlang:maintfrom
trollfred:binary_join
Closed

add binary:join/2#427
trollfred wants to merge 1 commit intoerlang:maintfrom
trollfred:binary_join

Conversation

@trollfred
Copy link
Copy Markdown

strings:join/2 for binaries!

strings:join/2 for binaries!
@OTP-Maintainer
Copy link
Copy Markdown

Patch has passed first testings and has been assigned to be reviewed


I am a script, I am not human


@fenollp
Copy link
Copy Markdown
Contributor

fenollp commented Jul 13, 2014

You should add tests along with your PRs.

A question to the OTP team: API-wise isn't it a good idea to have polymorphic functions?
string:join/2 would thus have a clause for lists and a clause for binaries.

@psyeugenic
Copy link
Copy Markdown
Contributor

@fenollp : The idea that we've kicked around here since EEP35 has been a string library that operates on io-lists instead of just binaries. Though I think it would be a good idea with an io-list string library, I also think it would be a high complexity cost .. unless we do iolist_to_binary on every input .. which kind of defeat it's purpose I guess.

@kittee : See EEP35 bstring:join/2. I don't know the status of EEP35, I don't think it's accepted (it lacks a reference implementation).

Edit: spelling

@bjorng
Copy link
Copy Markdown
Contributor

bjorng commented Sep 15, 2014

Thanks for your contribution, but we have decided to reject it because we don't think that join/2 fits in the binary module. join/2 belongs in a module with general string manipulation functions such as bstring as suggested in EEP35; the binary module contains more specialized functions that are highly optimized.

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.

5 participants