dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.16k stars 4.72k forks source link

[API Proposal]: Add DateTime support overload for ToDateTime in PersianCalendar #74713

Closed sajjadsaharkhan closed 2 years ago

sajjadsaharkhan commented 2 years ago

Background and motivation

hi. I'm using System.Globalization.PersianCalendar class in my code to convert Utc or system time zone date time to Persian date time. its have a method and gets a date-time from year , month ,day,... It pollutes my code.

var toDay = persianCalendar.ToDateTime(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day,
            DateTime.Now.Hour, DateTime.Now.Minute, DateTime.Now.Second, DateTime.Now.Millisecond);

// i prefer to use this code:

var toDay = persianCalendar.ToDateTime(DateTime.Now);

i think PersianCalendar need a new overload for ToDateTime api

API Proposal

namespace System.Globalization;

public class PersianCalendar : Calendar
{
    public DateTime ToDateTime(DateTime dateTime)
    {
        return ToDateTime(dateTime.Year, dateTime.Month, dateTime.Day,
            dateTime.Hour, dateTime.Minute, dateTime.Second, dateTime.Millisecond);
    }
}

API Usage

var persianCalendar = new PersianCalendar();
var toDay = persianCalendar.ToDateTime(DateTime.Now);

Alternative Designs

No response

Risks

No response

dotnet-issue-labeler[bot] commented 2 years ago

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.

sajjadsaharkhan commented 2 years ago

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/area-system-globalization

sajjadsaharkhan commented 2 years ago

@ericstj @dotnet/area-system-globalization

Clockwork-Muse commented 2 years ago

It's not there because this:

namespace System.Globalization;

public class PersianCalendar : Calendar
{
    public DateTime ToDateTime(DateTime dateTime)
    {
        return ToDateTime(dateTime.Year, dateTime.Month, dateTime.Day,
            dateTime.Hour, dateTime.Minute, dateTime.Second, dateTime.Millisecond);
    }
}

isn't correct and doesn't work. You can't encode calendars other than ISO/Gregorian into DateTime - for instance, DateTime will prevent day-of-month being higher than 29 for month 2 (and often it being 29 as well).

The anticipated usage pattern for the calendars is that you get the raw numeric values from whatever input source and immediately convert it to DateTime, then keep it there with the various PersianCalendar.AddXXX methods.

DateTime.Now is essentially "already" a Persian calendar date, because that's the anticipated unit of storage/transmission; it's during display/manipulation you need to use the calendars.

ghost commented 2 years ago

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

Issue Details
### Background and motivation hi. I'm using `System.Globalization.PersianCalendar` class in my code to convert Utc or system time zone date time to Persian date time. its have a method and gets a date-time from year , month ,day,... It pollutes my code. ```csharp var toDay = persianCalendar.ToDateTime(DateTime.Now.Year, DateTime.Now.Month, DateTime.Now.Day, DateTime.Now.Hour, DateTime.Now.Minute, DateTime.Now.Second, DateTime.Now.Millisecond); // i prefer to use this code: var toDay = persianCalendar.ToDateTime(DateTime.Now); ``` i think `PersianCalendar` need a new overload for `ToDateTime` api ### API Proposal ```csharp namespace System.Globalization; public class PersianCalendar : Calendar { public DateTime ToDateTime(DateTime dateTime) { return ToDateTime(dateTime.Year, dateTime.Month, dateTime.Day, dateTime.Hour, dateTime.Minute, dateTime.Second, dateTime.Millisecond); } } ``` ### API Usage ```csharp var persianCalendar = new PersianCalendar(); var toDay = persianCalendar.ToDateTime(DateTime.Now); ``` ### Alternative Designs _No response_ ### Risks _No response_
Author: sajjadsaharkhan
Assignees: -
Labels: `api-suggestion`, `area-System.Runtime`, `untriaged`
Milestone: -
sajjadsaharkhan commented 2 years ago

It's not there because this:

namespace System.Globalization;

public class PersianCalendar : Calendar
{
    public DateTime ToDateTime(DateTime dateTime)
    {
        return ToDateTime(dateTime.Year, dateTime.Month, dateTime.Day,
            dateTime.Hour, dateTime.Minute, dateTime.Second, dateTime.Millisecond);
    }
}

isn't correct and doesn't work. You can't encode calendars other than ISO/Gregorian into DateTime - for instance, DateTime will prevent day-of-month being higher than 29 for month 2 (and often it being 29 as well).

The anticipated usage pattern for the calendars is that you get the raw numeric values from whatever input source and immediately convert it to DateTime, then keep it there with the various PersianCalendar.AddXXX methods.

DateTime.Now is essentially "already" a Persian calendar date, because that's the anticipated unit of storage/transmission; it's during display/manipulation you need to use the calendars.

it's true and I got it thanks