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

Refactor: Append interpolated or concatenated string to Append calls #1562

Open
wmjordan opened this issue Oct 12, 2024 · 4 comments
Open

Refactor: Append interpolated or concatenated string to Append calls #1562

wmjordan opened this issue Oct 12, 2024 · 4 comments

Comments

@wmjordan
Copy link

wmjordan commented Oct 12, 2024

Given the following code:

var sb = new StringBuilder();
var var1 = DateTime.Now.ToString();

sb.Append($"ABC{var1}EFG");

The third line used string interpolation, which is unnecessary, can be refactored to a series of Append calls as the following code shows.

sb.Append("ABC").Append(var1).Append("EFG");

Moreover, it is also useful to change Append concatenated string to Append calls as well. For instance, the following can also be refactored to the above statement.

sb.Append("ABC" + var1 + "EFG");

// or

sb.Append(String.Concat("ABC", var1, "EFG"));
@wmjordan wmjordan changed the title Refactor: Append interpolated string to Append calls Refactor: Append interpolated or concatenated string to Append calls Oct 12, 2024
@josefpihrt
Copy link
Collaborator

Following code will use AppendInterpolatedStringHandler under the hood which is even better then StringBuilder.Append methods.

sb.Append($"ABC{var1}EFG");

SharpLab: https://sharplab.io/#v2:CYLg1APgAgTAjAWAFBQAwAIpwHQBUCmAHgC4Dcyys6AwsgN7LpOYAs6AsgBQCUjzDSZkPQA3AIYAndAGcARugC86AHb4A7ugDKxCQEtlAcwBCAV10AbYPgk9yg4U3FSncRegAiY4vly6AtvjYAHIA9mp4Idp6hrYU9g5y2ACCAA4p+MrAnAAkAERJRtR0LgC+AKIAYgDiudx2QiXIjUhAA==

Following code will be converted into previous case by RCS1267.

sb.Append(String.Concat("ABC", var1, "EFG"));

Following code could be added to RCS1267 as an improvement.

sb.Append("ABC" + var1 + "EFG");

@wmjordan
Copy link
Author

wmjordan commented Oct 12, 2024

Yes, I know about the AppendInterpolatedStringHandler thing.
However, in my practical situation, there are quite a few Append calls around this one. Thus the StringBuilder can not be simply substituted by a interpolated string. For example,

var sb = new StringBuilder();
var var1 = DateTime.Now.ToString();

for (var i = 0; i = 100; i++) {
    sb.Append(somethingElse(i));
}
sb.Append($"ABC{var1}EFG");

or sometimes, the sb.Append(interpolated string) is invoked within a loop. So, it is arguable that the string interpolation will bring ultimate benefit to the overall performance.

Leaving the AppendInterpolatedStringHandler thing there, will generate a lot of intermediate string instances, cause extra memory copying, and put burden to the GC. If we break it into Append calls, there will be no such burden then.

@josefpihrt
Copy link
Collaborator

Thus the StringBuilder can not be simply substituted by a interpolated string.

Not sure if I understand. Why it couldn't be substitued?

sb.Append(interpolated string) is invoked within a loop

Why it matters if it's in a loop or not?

will generate a lot of intermediate string instances, cause extra memory copying

Where do you see these allocations in the SharpLab example?

@wmjordan
Copy link
Author

You are right. From .NET 6 on, there is no significant performance degrade when calling StringBuilder.Append(interpolated string).

Sorry, I did not mentioned that the situation was that the code was compiled for the old .NET Framework where no AppendInterpolatedStringHandler exists. I checked the decompiled code from an assembly and the code is compiled as a call to String.Format, then StringBuilder.Append(string).

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

No branches or pull requests

2 participants