Skip to content
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

Use strings builder #46

Merged
merged 6 commits into from
Jun 10, 2020
Merged

Use strings builder #46

merged 6 commits into from
Jun 10, 2020

Conversation

ninedraft
Copy link

Hello! I'm suggesting to use the strings.Builder instead of bytes.Buffer. It's a more efficient alternative with very low cast costs.

@huandu
Copy link
Owner

huandu commented Jun 10, 2020

strings.Builder is available since go1.10. It can be a break change for packages or projects importing this package.

Please consider to implement a helper builder which calls strings.Builder if it's available and uses bytes.Buffer as a fallback.

@ninedraft
Copy link
Author

strings.Builder is available since go1.10. It can be a break change for packages or projects importing this package.

Please consider to implement a helper builder which calls strings.Builder if it's available and uses bytes.Buffer as a fallback.

done ✅

@huandu
Copy link
Owner

huandu commented Jun 10, 2020

Very cool! Thank you.

I have several comments on filenames of the newly added files.

  • According to Go best practice, when we need to build code for specific Go version, we should add version as suffix in a name like buffer_go110.go. Therefore, buffer.go and buffer_old.go should be renamed as buffer_go110.go and buffer.go respectively.
  • Filename should be consistence with type name. The buffer is not a right name as the type defined in this file is bufferString. BTW, I think it would be better to name the type as stringBuilder and the file as stringbuilder.go.

@ninedraft
Copy link
Author

Very cool! Thank you.

I have several comments on filenames of the newly added files.

* According to Go best practice, when we need to build code for specific Go version, we should add version as suffix in a name like `buffer_go110.go`. Therefore, `buffer.go` and `buffer_old.go` should be renamed as `buffer_go110.go` and `buffer.go` respectively.

* Filename should be consistence with type name. The `buffer` is not a right name as the type defined in this file is `bufferString`. BTW, I think it would be better to name the type as `stringBuilder` and the file as `stringbuilder.go`.

done ✅

@huandu huandu merged commit 52197b1 into huandu:master Jun 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants