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

Proposal: adding Append overloads with format and IFormatProvider to StringBuilder #50674

Closed
Wojmik opened this issue Apr 3, 2021 · 4 comments · Fixed by #51653
Closed

Proposal: adding Append overloads with format and IFormatProvider to StringBuilder #50674

Wojmik opened this issue Apr 3, 2021 · 4 comments · Fixed by #51653
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@Wojmik
Copy link

Wojmik commented Apr 3, 2021

Background and Motivation

StringBuilder is easy to use and powerful api. It is often used to serialize some data - eg. to text file. This data commonly have to be in InvariantCulture and have specific format.

Sadly currently there is no way to append primitive value types like double or DateTime with format and IFormatProvider without unnecessary allocation. You can use AppendFormat but value is object so there is a boxing. You can use Append but there is no format nor IFormatProvider parameter.

All existing Append methods use this method internally:

private StringBuilder AppendSpanFormattable<T>(T value) where T : ISpanFormattable

But there is already another overload of this method that can be used:

 internal StringBuilder AppendSpanFormattable<T>(T value, string? format, IFormatProvider? provider) where T : ISpanFormattable, IFormattable

I don't known if there is another boxing here because of interface use (ISpanFormattable) on struct by the way.

I think I can make these changes, it shouldn't be hard anyway.

Proposed API

Add the following overloads to StringBuilder:

Append(byte value, string? format, IFormatProvider? provider);
Append(sbyte value, string? format, IFormatProvider? provider);
Append(short value, string? format, IFormatProvider? provider);
Append(ushort value, string? format, IFormatProvider? provider);
Append(int value, string? format, IFormatProvider? provider);
Append(uint value, string? format, IFormatProvider? provider);
Append(long value, string? format, IFormatProvider? provider);
Append(ulong value, string? format, IFormatProvider? provider);
Append(float value, string? format, IFormatProvider? provider);
Append(double value, string? format, IFormatProvider? provider);
Append(decimal value, string? format, IFormatProvider? provider);
//Those are completely new
Append(DateTime value, string? format = null, IFormatProvider? provider = null);
Append(DateTimeOffset value, string? format = null, IFormatProvider? provider = null);
Append(TimeSpan value, string? format = null, IFormatProvider? provider = null);
Append(Guid value, string? format = null);

Usage Examples

StringBuilder sb = new(64);
sb.Append(DateTime.Today, "yyyy-MM-dd", CultureInfo.InvariantCulture);
sb.Append(double.Epsilon, null, CultureInfo.InvariantCulture);

Alternative Designs

Alternatively only format and IFormatProvider parameters with default null value can be added to existing Append methods but this would be breaking change probably.
Still overloads for DateTime, DateTimeOffset, TimeSpan and Guid need to be added.

Risks

More code unless using alternative design.

@Wojmik Wojmik added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Apr 3, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 3, 2021
@davidfowl
Copy link
Member

I think we'd just wait until ISpanFormattable was public then expose that internal method.

@stephentoub
Copy link
Member

See #50601 and #50635. No additional surface area would be necessary to support what you want, e.g. with your example:

sb.AppendFormat(CultureInfo.InvariantCulture, $"{DateTime.Today:yyyy-MM-dd}");
sb.AppendFormat(CultureInfo.InvariantCulture, $"{double.Epsilon}");

I don't known if there is another boxing here because of interface use (ISpanFormattable) on struct by the way.

There is not.

@Wojmik
Copy link
Author

Wojmik commented Apr 3, 2021

@stephentoub it looks fantastic. So I just wait for C# 10 - hopefully.
It would be great if those overloads be added to System.IO.TextWriter also. This abstract type has readonly FormatProvider field but it cannot be set in StreamWriter so it's not very useful.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 21, 2021
@tannergooding tannergooding removed the untriaged New issue has not been triaged by the area owner label Jul 12, 2021
@tannergooding tannergooding modified the milestones: 7.0.0, Future Jul 12, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Projects
None yet
5 participants