At/On overloads that return DateTimeOffset?

[hmansell]

The use of DateTime in our internal codebase is one of the most common sources of errors when code runs in a different timezone. We generally find DateTimeOffset to be pretty robust for these purposes. We currently have our own Date type, and we have an At method that takes a TimeSpan (for TimeOfDay) and a TimeZoneInfo, and returns a DateTimeOffset. It would be good to have that support in here. I'm happy to submit a PR if you think this makes sense.

Would also be good to discourage use of DateTime and encourage DateTimeOffset wherever possible.

[mattjohnsonpint]

Thanks for the feedback. On discouraging DateTime - I hear you, but also it's a slippery slope. While DateTimeOffset does have very many good use cases, there are also still good use cases for a DateTime - especially one with DateTimeKind.Unspecified.

One example is in domains such as television schedules - where a show airs (for example) at 2015-10-08T18:00:00 - but that might be four different instants in time when it's applied to the four major time zones in the continental USA. This is sometimes called a "floating time". There are other domains where floating time is required.

Besides the few business domains where that is a natural fit, also consider the much more general case of prompting a user for input from a form. You might ask for a date, or a time, or a date and time, but you'll rarely ask for an offset directly. Another common use case is when loading simple flat files, such as CSV. Rarely are offsets present in this sort of data. If you force a DateTimeOffset into these scenarios, then you force the decision of whether to infer local time or UTC much too early in the process.

Indeed, you will see this behavior today in that the local time zone is assumed in the DateTimeOffset constructor, and in the implicit cast that occurs if you try to assign a DateTime to a DateTimeOffset. That's a dangerous assumption (IMHO).

So - discouraging DateTime for all cases is not a good idea. However, I do agree with you that in many cases DateTimeOffset is better. The key distinguishing factor is that DateTimeOffset always tracks an exact moment in time, while DateTime might be referring to an exact moment in time or might have some other interpretation. See more (with a helpful analogy) in my reply to DateTime vs DateTimeOffset on StackOverflow.

... whew, that was long, ok on to the next part ...

As you pointed out, we already have Date.At(TimeOfDay) and TimeOfDay.At(Date). Both return a DateTime. Sure, logically we could add Date.At(TimeOfDay, TimeSpan) and TimeOfDay.On(Date, TimeSpan) to return DateTimeOffset values, but those seem rather awkward - especially the second one.

I think what what would make more sense is just a more discoverable way to go from DateTime to DateTimeOffset. A simple WithOffset extension method would be perfect for this. Then you could write Date.At(TimeOfDay).WithOffset(TimeSpan), and TimeOfDay.On(Date).WithOffset(TimeSpan). IMHO, both are clearer in intent than using the DateTimeOffset constructor. Does that make sense to you?

[hmansell]

In my experience with our (large-ish) codebase, allowing any use of DateTime is the slippery slope. I agree that there are some use cases of local date/times (though I don't totally buy the television schedule one), but use cases of instants in time are much more common.

The problem is that DateTime attempts to cover multiple use cases (including dates!) without a clear distinction. The Kind is a very subtle thing that is often overlooked. The fact that dates are indistinguishable from date/times at midnight is a travesty!

I definitely agree there are some dodgy behaviors in conversions, and there shouldn't be an implicit cast. Though it is pretty hard to remove that now!

I also agree it is inconvenient to have to specify an offset whenever a DateTimeOffset is created, but IME it is the only way of writing code that robustly represents instants. BTW we have found it more convenient/robust to construct using a TimeZoneInfo - e.g. date.At(tod, timeZoneInfo) - as it usually conveys better what you mean and deals better with daylight savings.

I would be happy with your proposed date.At(timeOfDay).WithOffset(timeSpan) as long as At() returned some time other than DateTime - i.e. a LocalDateTime or something like that. This makes it clear what it actually represents.

I guess noda time has it right, but seems overly complex for many use cases, and I hold out hope that the BCL can be made better. Perhaps @jskeet can provide some useful input.

[jskeet]

Well, my personal view is that once you've thought what you actually want to do, Noda Time should end up being at most as complex as DateTime. The philosophy is that we force you do to all the thinking you really need to do, but then try to express the result of the thinking naturally and clearly.

DateTime feels to me like the opposite philosophy - you can express yourself pretty vaguely, and most of the time it may even do what you want.

If you're looking for simplicity of coding without having to think about what exactly you want, then date/time isn't an amenable domain :(

[mattjohnsonpint]

I agree about the philosophies. In my view, if you want an API that leads you down the right path and tries to prevent you from doing the wrong thing, then please use Noda Time. It's quite solid. The trade-off is in two parts: 1) Some things will be more verbose, by design - you noted this about the complexity. 2) Some interoperability areas are difficult, or not possible - such as EF support. Hopefully something will be done about that eventually (see dotnet/efcore#242).

WRT the work being done here for System.Time, the current philosophy is to accommodate the four existing types (DateTime, DateTimeOffset, TimeSpan and TimeZoneInfo) without changing their design, and to add the two key missing types (Date and TimeOfDay) to give an alternative to abusing DateTime or TimeSpan for those purposes. We can add extension methods, casts, and helpers along the way, but we can't do much else because the existing classes are so entrenched.

What we can do is to include some Roslyn Analyzers to highlight common usage mistakes. This was discussed extensively in dotnet/corefx#2116, dotnet/corefx#700 and dotnet/corefx#626.

To put it more clearly, LocalDateTime only works well if you also have Instant and ZonedDateTime, such that you could accommodate all usages of DateTime considering DateTimeKind. We can't really do that without changing the philosophy, which at that point we might as well just use Noda Time.

WRT to date.At(tod, timeZoneInfo) - The only problem there is that the resulting object is a DateTimeOffset. It's not time zone aware. In Noda Time, we'd expect such an operation to return a ZonedDateTime, but we don't have such an object yet. We could consider adding one, but IMHO - that is the line where I would want to use Noda Time instead.

Looking into the future I can see some guidance somewhere on MSDN that says something to the effect of this:

The .NET Framework and .NET Core include a rudimentary set of APIs for working with dates and times. These are in the System namespace and include DateTime, DateTimeOffset, TimeSpan and TimeZoneInfo. You may also consider using the System.Time package, which adds Date and TimeOfDay types, or you might consider using an open source library such as Noda Time, which offers a more comprehensive solution.

[mattjohnsonpint]

BTW - I'm not completely against the idea of changing the philosophy to offer a comprehensive solution out of the box. It would indeed make System.Time look a lot more like Noda Time. It would also require much broader interest and foundational support from the core framework team. I don't have the bandwidth to do it alone.

The current proposal is more of a middle-ground compromise, rather than a best-of-breed solution. It would be interesting if anyone else out there had a different idea for a middle-ground approach, or an alternative viewpoint than Noda Time that was still comprehensive.

[hmansell]

Thanks. The issue with the "cover the simple cases" philosophy is that dates/times are passed around between our code and third party/OSS libraries. We can choose to use NodaTime in our codebase, but where we are using other libraries the common currency is the DateTime and so we end up converting back-and-forth. That leads people to just use DateTime to avoid the ceremony, and then the slippery slope begins.

That said, I agree it's a big project and perhaps analyzers can spot common errors, though it's not clear to me how complete that can be without some type-system-level info that captures the DateTimeKind.

WRT date.At(tod, timeZoneInfo) - the purpose is to avoid folks determining the offset themselves and potentially doing it wrong (e.g. hard-coding offsets and ignoring DST). DateTimeOffset can't do timezone-aware date arithmetic but it can represent that specific instant, the offset of which can be specified using a TZI. Perhaps it would lead the unsuspecting developer to thing that DateTimeOffset is timezone-aware.

[mattjohnsonpint]

So then, you would like to see all three of these, correct?

DateTime dt = date.At(timeOfDay);
DateTimeOffset dto = date.At(timeOfDay, offset);
DateTimeOffset dto = date.At(timeOfDay, tzi);

And these too?

DateTime dt = timeOfDay.On(date);
DateTimeOffset dto = timeOfDay.On(date, offset);
DateTimeOffset dto = timeOfDay.On(date, tzi);

How would you feel about these?

DateTimeOffset dto = dateTime.WithOffset(offset);
DateTimeOffset dto = dateTime.InTimeZone(tzi);

The issue with either of those last two is that they'll suffer from the same problem we have in the current API. That is, whenever you go from DateTime to DateTimeOffset, you have to evaluate Kind.

[mattjohnsonpint]

The current APIs for that are:

DateTimeOffset dto = dateTime;
DateTimeOffset dto = new DateTimeOffset(dateTime);
DateTimeOffset dto = new DateTimeOffset(dateTime, offset);

The first one is a big problem for a lot of these APIs. While implicit casting helps for some things, it's not so great when it pulls in additional information. In this case, is pulls in the local time zone when kind is local or unspecified.

The second one is more explicit, but still has the same behavior with regard to using the local time zone.

The third one is ideal when kind is unspecified, but otherwise the kind has to match the offset or you get a runtime exception.

Another tricky API that we have to remember is:

TimeSpan offset = timeZoneInfo.GetUtcOffset(dateTime);

Here, if the kind is unspecified then it's interpreted to belong to the time zone. But if it is local or UTC, it's first adjusted to that time zone - essentially treating it as an Instant. This does cause some grief...

[mattjohnsonpint]

This will need to be re-thought with respect to #1901. Clearly there will be need for all of the following scenarios:

• Date without offset + Time without offset = DateTime with DateTimeKind.Unspecified
• Date without offset + Time with offset = DateTimeOffset
• Date with offset + Time without offset = DateTimeOffset
• Date + Time both with same offset = DateTimeOffset

And perhaps also:

• Date without offset + Time with Z = DateTime with DateTimeKind.Utc
• Date with Z offset + Time without offset = DateTime with DateTimeKind.Utc
• Date + Time both with Z offset = DateTime with DateTimeKind.Utc

[YohDeadfall]

Date without offset + Time without offset = DateTime with DateTimeKind.Unspecified

Why not DateTimeOffset with zero offset? The DateTime property of DateTimeOffset returns DateTime with DateTimeKind.Unspecified (source). So if someone wants DateTime then he should use date.At(time).DateTime.

[mattjohnsonpint]

I don't agree, we shouldn't make the presumption that because an offset was not provided that the offset is zero. That is incorrectly mapping an unspecified (abstract, floating, civil, etc.) value to a specific point in time.

[YohDeadfall]

So how should this situation be solved? By an additional method for that case only?

[mattjohnsonpint]

Perhaps. I'm open to ideas. I will experiment with a few things myself and report back here. Thanks. :)

[jzabroski]

Hi,

I feel there are not enough use cases in this discussion. Perhaps the strawman presented by @mj1856 is truly how it needs to be, but:

1. I think the fundamental issue is Kind is hidden internal to the DateTime structure, as opposed to exposing DateTimeKindUnspecified as a first-class type. This is basically the direction java 8 went in with java.time and java.time.temporal.

The issue with the "cover the simple cases" philosophy is that dates/times are passed around between our code and third party/OSS libraries. We can choose to use NodaTime in our codebase, but where we are using other libraries the common currency is the DateTime and so we end up converting back-and-forth. That leads people to just use DateTime to avoid the ceremony, and then the slippery slope begins.

2. In terms of use cases, @hmansell brings up an excellent point that libraries usually only support DateTime, and so whatever solution comes up, there has to be a way to generate thunks for calling the API.
3. One use case I can think of is Date controls that represent a logical business day as a DateTime object instead of just a Date.
4. The other problem is consuming bad APIs written in Java that auto-serialize Oracle DateTime database columns as java:java.util.Date. If a java API looks purely at an Oracle Date column and doesn't realize the value is always the result of calling TRUNC(
   date), the auto serialization will add timezone info.

Would also be good to discourage use of DateTime and encourage DateTimeOffset wherever possible.

I think, in order to do that, you need to contact textbook authors, because the problem starts in the classroom. Java has completely forced the issue by creating a big promotion of this as part of Java 8.

BTW - I'm not completely against the idea of changing the philosophy to offer a comprehensive solution out of the box. It would indeed make System.Time look a lot more like Noda Time. It would also require much broader interest and foundational support from the core framework team. I don't have the bandwidth to do it alone.

I think you might get inspiration from Ben Evan's and Richard Warburton's article describing the motivation for cleaning up date and time in Java:
http://www.oracle.com/technetwork/articles/java/jf14-date-time-2125367.html

I think, if there is ever a motivation for core team to fix something, the idea that "java does it better than you" (appeal to ego), might be it!

One example is in domains such as television schedules - where a show airs (for example) at 2015-10-08T18:00:00 - but that might be four different instants in time when it's applied to the four major time zones in the continental USA. This is sometimes called a "floating time". There are other domains where floating time is required.

I am not as smart as you, but for the life of me, I cannot fathom why such logic would not be encapsulated in a DateTimeFloat data type if we're supposed to represent something that can float across time zones. I'm just very into domain driven design, and what you're telling me is you don't practice DDD and it's ok to you that your types don't have precise domain meaning.

[jzabroski]

Perhaps. I'm open to ideas. I will experiment with a few things myself and report back here. Thanks. :)

Just remove DateTime from the SDK altogether. Forget trying to fix it. Just banish it. Is this not an option?