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 TryFormat to IFormattable as a default interface method #30547

Closed
Gnbrkm41 opened this issue Aug 9, 2019 · 1 comment · Fixed by #51086
Closed

Proposal: Adding TryFormat to IFormattable as a default interface method #30547

Gnbrkm41 opened this issue Aug 9, 2019 · 1 comment · Fixed by #51086
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@Gnbrkm41
Copy link
Contributor

Gnbrkm41 commented Aug 9, 2019

Related: https://github.com/dotnet/corefx/issues/30114 (Discussion about introducing a new interface that has TryFormat)

Rationale

TryFormat is as a less-allocating alternative of ToString, which helps us reduce heap allocations by using provided reusable buffers instead of creating a new immutable string every time by calling ToString. However, since it isn't part of any interface/classes, there is no way that people can use it in APIs without hard-coding the type which are known to have TryFormat.

Now that we have Default Interface Methods added to C# 8 (and having it supported in .NET Standard 2.1+ / .NET Core 3.0+), I think it would be great if we can add TryFormat to IFormattable interface as a default interface method, that falls back to the implementation of ToString if not implemented. The allocation caused by fallback if not implemented wouldn't likely be an issue, since there would be no alternatives but to call ToString, which would allocate the same amount of memory. Also, it seems pretty obvious that the behaviour of TryFormat when passed the same format/content should be the same as the one from equivalent ToString call, so calling ToString as a fallback would make sense.

Proposed API

namespace System
{
    public interface IFormattable
    {
        string ToString(string? format, IFormatProvider? formatProvider);
+       bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format, IFormatProvider? provider);
    }
}

There is no implementations given in this proposal; However it would be very trivial, calling ToString then attempting to copy the contents of the returned string into the provided Span.

Questions

Would it be worth making format and provider parameters optional for occasions when you just want it to return whatever is the default, given that there isn't a parameter-less TryFormat unlike object.ToString?

@Gnbrkm41 Gnbrkm41 changed the title Proposal: Adding TryFormat to IFormattable as a default implemented method Proposal: Adding TryFormat to IFormattable as a default interface method Aug 9, 2019
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the Future milestone Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@joperezr joperezr added api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Jul 2, 2020
@john-h-k
Copy link
Contributor

john-h-k commented Feb 2, 2021

Sometimes it is useful to know if the TryFormat method is "real" or just forwards to ToString - that is an advantage of it being on ISpanFormattable, however, you could expose bool TryFormatIsFast => false DIM or similar. But it's a bit ugly

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 11, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Apr 15, 2021
@ghost ghost locked as resolved and limited conversation to collaborators May 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants