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

[API Proposal]: new System.ComponentModel.TimeOnlyConverter class #68744

Closed
0xced opened this issue May 1, 2022 · 3 comments
Closed

[API Proposal]: new System.ComponentModel.TimeOnlyConverter class #68744

0xced opened this issue May 1, 2022 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.ComponentModel

Comments

@0xced
Copy link
Contributor

0xced commented May 1, 2022

Background and motivation

Converting between System.TimeOnly and string is currently not supported using a System.ComponentModel.TypeConverter. Many system types are supported out of the box (System.DateTimeOffset, System.Guid, System.TimeSpan, System.Uri etc.) so I think System.TimeOnly would be a welcome addition.

I was expecting this to work, but converting from string actually throws:

System.NotSupportedException: TypeConverter cannot convert from System.String.

TypeConverter timeOnlyConverter = TypeDescriptor.GetConverter(typeof(TimeOnly));
TimeOnly? time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

Having built-in support for type conversion would probably help to drive adoption of the relatively new TimeOnly type.

Addressing this issue would fix #59253 which is mentioned in System.Runtime work planned for .NET 7 (but only under the Backlog list, not the Planned for .NET 7 list).

Note that a similar proposal for the System.DateOnly type has been filed as #68743.

API Proposal

public class TimeOnlyConverter : System.ComponentModel.TypeConverter
{
    public override bool CanConvertFrom(System.ComponentModel.ITypeDescriptorContext? context, System.Type sourceType);
    public override bool CanConvertTo(System.ComponentModel.ITypeDescriptorContext? context, System.Type? destinationType);
    public override object? ConvertFrom(System.ComponentModel.ITypeDescriptorContext? context, System.Globalization.CultureInfo? culture, object value);
    public override object? ConvertTo(System.ComponentModel.ITypeDescriptorContext? context, System.Globalization.CultureInfo? culture, object? value, System.Type destinationType);
    public override bool IsValid(System.ComponentModel.ITypeDescriptorContext? context, object? value);
}

API Usage

Using the converter would most likely happen through TypeDescriptor.GetConverter(typeof(TimeOnly)):

var timeOnlyConverter = TypeDescriptor.GetConverter(typeof(TimeOnly));
var time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

or even through a higher level abstractions such as Microsoft.Extensions.Configuration.Binder (which also uses TypeDescriptor.GetConverter internally) as described in #64739:

var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string, string> { ["BreakfastTime"] = "7:00:00" })
    .Build();
var breakfastTime = configuration.GetValue<TimeOnly>("BreakfastTime");

It could also be used by directly creating a TimeOnlyConverter instance but this is a less likely scenario:

var timeOnlyConverter = new System.ComponentModel.TimeOnlyConverter();
var time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

Alternative Designs

To the best of my knowledge, there's no alternative design that would make sense for adding a new System.ComponentModel.TypeConverter.

Risks

System.ComponentModel.TimeOnlyConverter being a new class, no risks are introduced.

@0xced 0xced added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 1, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.ComponentModel untriaged New issue has not been triaged by the area owner labels May 1, 2022
@ghost
Copy link

ghost commented May 1, 2022

Tagging subscribers to this area: @dotnet/area-system-componentmodel
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Converting between System.TimeOnly and string is currently not supported using a System.ComponentModel.TypeConverter. Many system types are supported out of the box (System.DateTimeOffset, System.Guid, System.TimeSpan, System.Uri etc.) so I think System.TimeOnly would be a welcome addition.

I was expecting this to work, but converting from string actually throws:

System.NotSupportedException: TypeConverter cannot convert from System.String.

TypeConverter timeOnlyConverter = TypeDescriptor.GetConverter(typeof(TimeOnly));
TimeOnly? time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

Having built-in support for type conversion would probably help to drive adoption of the relatively new TimeOnly type.

Addressing this issue would fix #59253 which is mentioned in System.Runtime work planned for .NET 7 (but only under the Backlog list, not the Planned for .NET 7 list).

Note that a similar proposal for the System.DateOnly type has been filed as #68743.

API Proposal

public class TimeOnlyConverter : System.ComponentModel.TypeConverter
{
    public override bool CanConvertFrom(System.ComponentModel.ITypeDescriptorContext? context, System.Type sourceType);
    public override bool CanConvertTo(System.ComponentModel.ITypeDescriptorContext? context, System.Type? destinationType);
    public override object? ConvertFrom(System.ComponentModel.ITypeDescriptorContext? context, System.Globalization.CultureInfo? culture, object value);
    public override object? ConvertTo(System.ComponentModel.ITypeDescriptorContext? context, System.Globalization.CultureInfo? culture, object? value, System.Type destinationType);
    public override bool IsValid(System.ComponentModel.ITypeDescriptorContext? context, object? value);
}

API Usage

Using the converter would most likely happen through TypeDescriptor.GetConverter(typeof(TimeOnly)):

var timeOnlyConverter = TypeDescriptor.GetConverter(typeof(TimeOnly));
var time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

or even through a higher level abstractions such as Microsoft.Extensions.Configuration.Binder (which also uses TypeDescriptor.GetConverter internally) as described in #64739:

var configuration = new ConfigurationBuilder()
    .AddInMemoryCollection(new Dictionary<string, string> { ["BreakfastTime"] = "7:00:00" })
    .Build();
var breakfastTime = configuration.GetValue<TimeOnly>("BreakfastTime");

It could also be used by directly creating a TimeOnlyConverter instance but this is a less likely scenario:

var timeOnlyConverter = new System.ComponentModel.TimeOnlyConverter();
var time = timeOnlyConverter.ConvertFromString("7:00") as TimeOnly?;

Alternative Designs

To the best of my knowledge, there's no alternative design that would make sense for adding a new System.ComponentModel.TypeConverter.

Risks

System.ComponentModel.TimeOnlyConverter being a new class, no risks are introduced.

Author: 0xced
Assignees: -
Labels:

api-suggestion, area-System.ComponentModel, untriaged

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented May 1, 2022

@0xced please move this proposal with the one in #68743. Feel free to close this one after merging the proposals. Thanks!

@0xced
Copy link
Contributor Author

0xced commented May 1, 2022

I have just amended #68743 to cover both DateOnly and TimeOnly, so I'm closing this one.

@0xced 0xced closed this as completed May 1, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 1, 2022
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.ComponentModel
Projects
None yet
Development

No branches or pull requests

2 participants