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

Adding fractional milliseconds (microseconds) to a DateTime or TimeSpan not working #23771

Closed
ChristopherHaws opened this issue Oct 8, 2017 · 9 comments
Milestone

Comments

@ChristopherHaws
Copy link

DateTime.AddMilliseconds and TimeSpan.FromMilliseconds currently take a Double as the parameter, however when passing a fractional value either one, they ignore the numbers after the decimal place. To get around this, I had to create an extension methods to add microseconds manually. With the current API surface, there are only two ways to add microseconds; using Parse or adding them directly to the Ticks.

Versions Used:
.NET Framework 4.7
.NET Core 2.0

Steps to Reproduce:
Create a new console application with the following code:

void Main()
{
    var a = DateTime.Parse("0001-01-01 00:00:00.1234567");
    var b = new DateTime(0).AddMilliseconds(123.4567d);
    var c = new DateTime(0).AddMilliseconds(123).AddMicroseconds(4567);

    var x = TimeSpan.Parse("00:00:00.1234567");
    var y = TimeSpan.FromMilliseconds(123.4567d);
    var z = TimeSpan.FromMilliseconds(123).AddMicroseconds(4567);
    
    Console.WriteLine(a.Ticks); // 1234567
    Console.WriteLine(b.Ticks); // 1230000
    Console.WriteLine(c.Ticks); // 1234567

    Console.WriteLine(x.Ticks); // 1234567
    Console.WriteLine(y.Ticks); // 1230000
    Console.WriteLine(z.Ticks); // 1234567
}

// Define other methods and classes here
public static class Extensions
{
    public static DateTime AddMicroseconds(this DateTime datetime, Int32 value)
    {
        return new DateTime(datetime.Ticks + value, datetime.Kind);
    }


    public static TimeSpan AddMicroseconds(this TimeSpan timespan, Int32 value)
    {
        return new TimeSpan(timespan.Ticks + value);
    }
}

Expected Behavior:
Values a, b, c, x, y, and z should all have the same Ticks.

DateTime.AddMilliseconds(123.4567d) should add 1234567 ticks to the DateTime.
TimeSpan.FromMilliseconds(123.4567d) should add 1234567 ticks to the TimeSpan.

Actual Behavior:
DateTime.AddMilliseconds(123.4567d) only adds 123 milliseconds to the DateTime, ignoring the 4567 microseconds.
TimeSpan.FromMilliseconds(123.4567d) only adds 123 milliseconds to the TimeSpan, ignoring the 4567 microseconds.

@stephentoub
Copy link
Member

stephentoub commented Oct 8, 2017

The docs for AddMilliseconds state "The value parameter is rounded to the nearest integer", so this appears to be by design... though I don't know why that's the design.

@danmoseley
Copy link
Member

Maybe @tarekgh happens to know. I guess this can be closed by design, unless we think it would be an acceptable change to "fix" it.

@tarekgh
Copy link
Member

tarekgh commented Oct 9, 2017

Right. this is the current design how we round the values.
We have tried to fix a such/similar issue before dotnet/coreclr#10352 but caused other app compat issues so we decided to keep the current for the sake of app compat.

@tarekgh tarekgh closed this as completed Oct 9, 2017
@ChristopherHaws
Copy link
Author

Would it maybe be reasonable to have a way to get/set the lower precision values? The fact that the only way to view the lower precision numbers is by using the ticks and making extension methods it a bit cumbersome. I came across this when writing some unit tests and from the logging and debugger, the two values I was comparing looked identical down to the millisecond, but they were different ticks so they were microseconds apart. Would be nice to have some additional methods. Seems like this could be done in a minor version since it would be adding API surface, not changing existing API's, and functionality would be unchanged.

It would also be nice for ToString to include micro and nano seconds if they aren't zero, or at the very least, the debugger display.

Thanks

@tarekgh
Copy link
Member

tarekgh commented Oct 10, 2017

It would also be nice for ToString to include micro and nano seconds if they aren't zero, or at the very least, the debugger display.

You already can do that. Please check the following link:

https://docs.microsoft.com/en-us/dotnet/standard/base-types/custom-date-and-time-format-strings#fffffffSpecifier

the two values I was comparing looked identical down to the millisecond, but they were different ticks so they were microseconds apart.

does the comparing of the formatted dates (with fffffff pattern) would help in this case? did you try it?

@ChristopherHaws
Copy link
Author

Yes, we are doing that, it's just not ideal. See what I had to do in this PR to add support for fractional seconds to FluentAssertions: https://github.com/fluentassertions/fluentassertions/pull/669/files

This file in particular has methods that to me seems like they should exist in .NET: TimeSpanConversionExtensions.cs

@tarekgh
Copy link
Member

tarekgh commented Oct 10, 2017

@ChristopherHaws you can log a new issue with the proposal of the new APIs you want to add and we can discuss the details. (here is issue example you can follow to file yours https://github.com/dotnet/corefx/issues/24449)

For ToString() I don't think we can change the default behavior for the app compatibility reason and we already supporting the patterns that can help show the micro, mini...seconds. so I guess there are no more requirements there. So, I guess what you want to propose is APIs allow adding or setting micro, nano..seconds.

@ChristopherHaws
Copy link
Author

ChristopherHaws commented Oct 10, 2017

Thanks. I created an issue for that: https://github.com/dotnet/corefx/issues/24555

@tomas-andersson
Copy link

Yes, we are doing that, it's just not ideal. See what I had to do in this PR to add support for fractional seconds to FluentAssertions: https://github.com/fluentassertions/fluentassertions/pull/669/files

For those who does not have time to follow the link, this is the code that adds "support for fractional seconds".

public static TimeSpan Seconds(this double seconds)
{
return TimeSpan.FromSeconds(seconds);
}

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants