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

Overhaul DateTimeConverter #765

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Rob-Hague
Copy link
Contributor

@Rob-Hague Rob-Hague commented Mar 25, 2023

This PR is a more elaborate (overboard?) change than #764. The changes are summarised as:

  1. Automatically parse arbitrary fractional seconds precision. Removes the ConvertTo[..](string str, TimeStampPrecision precision) methods.
  2. Improve UTC offset parsing (fixes positive offset parsing and allows HH:mm format). Adds methods which return offset information when present (see below)
  3. Add methods for .NET 6 DateOnly and TimeOnly types for potential future use by DateOnlyField, TimeOnlyField etc.

API changes:

-DateTime ConvertToDateTime(string str, TimeStampPrecision precision) // Precision is parsed automatically
-DateTime ConvertToTimeOnly(string str, TimeStampPrecision precision)

// These methods store offset information in their out parameters if it is present
+DateTime ConvertToDateTime(ReadOnlySpan<char> str, out DateTimeOffset? dateTimeOffset)
+TimeOnly ConvertToTimeOnly(ReadOnlySpan<char> str, out TimeSpan? offset)

+DateOnly ConvertToDateOnly(ReadOnlySpan<char> str)
+string ConvertDateOnly(DateOnly date)
+string ConvertTimeOnly(TimeOnly time)
+string ConvertTimeOnly(TimeOnly time, bool includeMilliseconds)
+string ConvertTimeOnly(TimeOnly time, TimeStampPrecision precision)

Some now unused public constants and fields are also removed.

Behavioural changes:

(1) and (2) above together allow offset parsing at any precision of fractional seconds: currently the offset is only parsed in the ConvertFromNanoString method which is only reached for long enough input string length (>24):

if (str.Length > DATE_TIME_MAXLENGTH_WITHOUT_NANOSECONDS)
{
return DateTimeFromNanoString(str);
}
else
{
return System.DateTime.ParseExact(str, DATE_TIME_FORMATS, DATE_TIME_CULTURE_INFO, DATE_TIME_STYLES);
}

Additionally,

  • On master, the DateTime value from a string without offset information and up to microsecond precision has DateTime.Kind equal to DateTimeKind.Utc. In this PR it equals DateTimeKind.Unspecified.
  • On master, the DateTime value from a string without offset information and more than microsecond precision has DateTime.Kind equal to DateTimeKind.Local. In this PR it equals DateTimeKind.Unspecified.
  • On master, the DateTime value from a string with a negative UTC offset is adjusted to the UTC value but has DateTime.Kind equal to DateTimeKind.Unspecified. In this PR it equals DateTimeKind.Utc.
  • On master, the DateTime value from a string with a positive UTC offset is adjusted in the wrong direction. In this PR the value is adjusted to the UTC value with DateTime.Kind equal to DateTimeKind.Utc. (Fix positive UTC offset parsing in DateTimeConverter #764 is a targeted fix for this)
Input Result (master branch) Result (PR branch)
"20230325-14:00:00" 2023-03-25 14:00:00.0000000 (Utc) 2023-03-25 14:00:00.0000000 (Unspecified)
"20230325-14:00:00.123" 2023-03-25 14:00:00.1230000 (Utc) 2023-03-25 14:00:00.1230000 (Unspecified)
"20230325-14:00:00.1234567" 2023-03-25 14:00:00.0012345 (Local) 2023-03-25 14:00:00.1234567 (Unspecified)
"20230325-14:00:00.123456789" 2023-03-25 14:00:00.1234567 (Local) 2023-03-25 14:00:00.1234567 (Unspecified)
"20230325-14:00:00Z" QuickFix.FieldConvertError 2023-03-25 14:00:00.0000000 (Utc)
"20230325-14:00:00.123Z" QuickFix.FieldConvertError 2023-03-25 14:00:00.1230000 (Utc)
"20230325-14:00:00.1234567Z" 2023-03-25 14:00:00.0012345 (Utc) 2023-03-25 14:00:00.1234567 (Utc)
"20230325-14:00:00+5" QuickFix.FieldConvertError 2023-03-25 09:00:00.0000000 (Utc)
"20230325-14:00:00.123+5" QuickFix.FieldConvertError 2023-03-25 09:00:00.1230000 (Utc)
"20230325-14:00:00.1234567+5" 2023-03-25 19:00:00.0012345 (Unspecified) 2023-03-25 09:00:00.1234567 (Utc)
"20230325-14:00:00.123456789+5" 2023-03-25 19:00:00.1234567 (Unspecified) 2023-03-25 09:00:00.1234567 (Utc)
"20230325-14:00:00.123456789-5" 2023-03-25 19:00:00.1234567 (Unspecified) 2023-03-25 19:00:00.1234567 (Utc)
"20230325-14:00:00.123-5:30" QuickFix.FieldConvertError 2023-03-25 19:30:00.1230000 (Utc)

The behaviour of the two methods with out parameters is hopefully clear (and agreeable) from the documentation and the tests (which achieve 100% coverage). Here are examples. Note the difference between the two in that ConvertToTimeOnly does not adjust the return value by the offset (because TimeOnly does not have an equivalent to DateTimeKind)

ConvertToDateTime

  • For the input "20230325-14:00:00", the return value is equal to new DateTime(2023, 03, 25, 14, 00, 00, DateTimeKind.Unspecified) and the dateTimeOffset parameter is null
  • For the input "20230325-14:00:00-5", the return value is equal to new DateTime(2023, 03, 25, 19, 00, 00, DateTimeKind.Utc) and the dateTimeOffset parameter is equal to new DateTimeOffset(2023, 03, 25, 14, 00, 00, TimeSpan.FromHours(-5))

ConvertToTimeOnly

  • For the input "14:00:00", the return value is equal to new TimeOnly(14, 00, 00) and the offset parameter is null
  • For the input "14:00:00-5", the return value is also equal to new TimeOnly(14, 00, 00) and the offset parameter is equal to TimeSpan.FromHours(-5)

Where before the conversion could lose information pertaining to the offset, now no information is lost. Perhaps in the future DateTimeField and TimeOnlyField could have corresponding offset properties exposing this information.

1. Automatically parse arbitrary fractional seconds precision. Removes the
ConvertTo[..](string str, TimeStampPrecision precision) methods.

2. Improve UTC offset parsing (fixes positive offset parsing and allows HH:mm
format). Adds methods which return offset information when present.

3. Add methods for .NET 6 DateOnly and TimeOnly types.
@Rob-Hague
Copy link
Contributor Author

I must have been really bored when I wrote this.

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

Successfully merging this pull request may close these issues.

1 participant