Skip to content

Add use_string_buffers to the core or recommended sets #784

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

Closed
Levi-Lesches opened this issue Oct 26, 2022 · 6 comments
Closed

Add use_string_buffers to the core or recommended sets #784

Levi-Lesches opened this issue Oct 26, 2022 · 6 comments

Comments

@Levi-Lesches
Copy link

Using string buffers instead of += can save a lot of time and space, as seen in this issue: dart-lang/sdk#50304. StringBuffer isn't necessarily ubiquitous, so the lint that tells you to use it should be enabled by default, either in core or recommended.

@natebosch
Copy link
Member

I worry a bit about recommending this one. The false positives may be rare, but I like to avoid lints with legitimate false positives that don't have any other mechanism than an // ignore. dart-lang/sdk#57777

I know @kevmoo is a fan of this lint. It does help more often than it hurts.

@kevmoo
Copy link
Member

kevmoo commented Nov 15, 2022

It keeps folks from using myString = myString + 'more stuff'; – which has HORRIBLE performance implications in Dart (I forgot who recently talked about how V8 has special hacks for this – @mraleph ?)

@kevmoo
Copy link
Member

kevmoo commented Nov 15, 2022

Ha! It was the referenced issue. 🤦

@kevmoo
Copy link
Member

kevmoo commented Nov 15, 2022

Short version: yes, I'm a fan.

I think // ignore is fine for one-offs. it forces folks to document why they avoid the best practice IMHO

@lrhn
Copy link
Member

lrhn commented Jun 7, 2023

It keeps folks from using myString = myString + 'more stuff'; – which has HORRIBLE performance implications in Dart

It's only a problem if you do it inside a loop, or do it repeatedly with long strings. Otherwise the overhead of allocating a StringBuffer with an internal growable list (two more objects) and six other fields, can very quickly cost more than just copying the bytes of myString once or twice.

The lint description is (as usual) very terse and doesn't say where it applies. If it applies outside of loops, and not just for more than n repeated additions to the same string in linear code, then it's simply too strict. Using String.operator+ is OK in some cases.

@devoncarew
Copy link
Member

After discussion, we think this lint will have too many false positives to add to package:lints. We would want this to only trigger for performance sensitive sections of code, but its not clear how we could successfully lint for that.

@mosuem mosuem transferred this issue from dart-archive/lints Dec 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

7 participants