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

feat: replacing bytes.Buffer instances used only to generate strings with strings.Builder. #224

Merged
merged 1 commit into from
Oct 11, 2022

Conversation

ellisonleao
Copy link
Contributor

@ellisonleao ellisonleao commented Oct 11, 2022

Ref #223

@ellisonleao ellisonleao changed the title feat: replacing bytes.Buffer instances used only to generate strings with strings.Builder. Ref #223 feat: replacing bytes.Buffer instances used only to generate strings with strings.Builder. Oct 11, 2022
@coveralls
Copy link

coveralls commented Oct 11, 2022

Coverage Status

Coverage increased (+4.0e-05%) to 99.861% when pulling 44305f9 on ellisonleao:issue-223 into 06e96ce on maxatome:master.

Copy link
Owner

@maxatome maxatome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, thanks for your PR!

a few small things:

  • could you also handle:
    • internal/json/lex.go:267
    • internal/util/string.go:83 renaming SliceToBuffer to SliceToString
  • could you change your commit message to
    refactor: string generation uses strings.Builder when possible
    
    Closes #224
    

@ellisonleao
Copy link
Contributor Author

done @maxatome

@maxatome
Copy link
Owner

maxatome commented Oct 11, 2022

My bad, by renaming SliceToBuffer to SliceToString I also meant changing the signature from using *bytes.Buffer to *strings.Builder. Sorry.

@ellisonleao
Copy link
Contributor Author

@maxatome any suggestions on how to handle buf.Truncate() and buf.Bytes(), since they don't exist in strings.Builder?

@maxatome
Copy link
Owner

  • buf.Bytes() is used by bytes.LastIndexByte so it can be replaced by buf.String() and strings.LastIndexByte;
  • buf.Truncate() call can be avoided by reworking the preceding loop:
    buf.WriteString(IndentString(ToString(items[0]), prefix))
    for _, item := range items[1:] {
      buf.WriteString(",\n")
      buf.WriteString(prefix)
      buf.WriteString(IndentString(ToString(item), prefix))
    }
    (not tested :) )

@ellisonleao
Copy link
Contributor Author

done @maxatome

@maxatome maxatome merged commit a56657c into maxatome:master Oct 11, 2022
@maxatome
Copy link
Owner

Thanks a lot @ellisonleao ! 😉 And welcome as go-testdeep contributor.

@ellisonleao ellisonleao deleted the issue-223 branch October 11, 2022 21:35
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.

None yet

3 participants