Noda Time is nearing v1.0, which means I’m spending more time writing documentation than code. It also means reviewing the APIs we’ve got with a critical eye – whether that’s removing extraneous members, adding useful ones, or moving things around. (In particular, writing documentation often suggests where a change would make calling code read more naturally.)
This post is about one particular section of the API, and the choices available. Although I do go into some detail around the specific calls involved, that’s just for context… the underlying choices are ones which could be faced when designing any API. I’ve rarely spent as much time thinking about API decisions as I have with Noda Time, so hopefully this will prove interesting to you even if you really don’t care about Noda Time itself as a project.
Introduction: time zones, local date/times and zoned date/times – oh my!
(Okay, so that’s not quite as snappy as the Judy Garland version, but hey…)
The area of API we’re going to focus on is time zones, and converting between "local" date/time values and "zoned" ones. The three types involved are:
- LocalDateTime: a "local" date and time, with no specific time zone. So, something like "7:30 in the evening on February 27th 2012". This means different instants in time in different time zones, of course: if you’re arranging a meeting, it’s good enough when the attendees are in the same time zone, but not good enough if you’re meeting with someone on the other side of the world. (A LocalDateTime also has an associated calendar system, but I’m not going to talk about that much in this post.)
- DateTimeZone: a time zone. At its core, this maps any "instant" in time to an offset – the difference between UTC and local time in that time zone. The offset changes over time, typically (but not always) due to daylight saving changes.
- ZonedDateTime: a date and time in a particular time zone, with an offset from UTC to avoid ambiguity in some cases (and for efficiency). This identifies a specific instant in time (simply by subtracting the offset from the local date/time). Conceptually this is equivalent to just maintaining the "instant" value, the time zone, and the calendar system – but it’s generally cleaner to think of it as a "local" value with an offset from UTC.
If those brief descriptions don’t make sense for you at the moment (this sort of thing is quite hard to describe concisely and precisely) you may want to see whether the Noda Time user guide "concepts" page helps.
The API task: mapping from { LocalDateTime, DateTimeZone } to ZonedDateTime
It’s easy to get from a ZonedDateTime to a LocalDateTime – you can just use the LocalDateTime property. The difficult bit is the other way round. We obviously want to be able to create a ZonedDateTime from the combination of a LocalDateTime and a DateTimeZone, but the question is where to put this functionality. Three options suggest themselves:
- A static method (or constructor) in ZonedDateTime which takes both the time zone and the local date/time as arguments
- An instance method on LocalDateTime which just takes the time zone as an argument
- An instance method on DateTimeZone which just takes the local date/time as an argument
It gets more complicated though – we’re not really talking about one operation here, but potentially several. Although the mapping from instant to offset is unambiguous in DateTimeZone, the mapping from LocalDateTime to offset is not straightforward. There can be 0, 1 or 2 possible results. For example, in the America/Los_Angeles time zone the clocks go forward from 2am to 3am on Sunday March 11th 2012, and go back from 2am to 1am on Sunday 4th November 2012. That means:
- The mapping from local date/time to offset at 7.30pm on February 27th 2012 is unambiguous: it’s definitely -8 hours (L.A. is 8 hours behind UTC).
- The mapping at 2.30am on March 11th 2012 is impossible: at 2am the clocks were put forward to 3am, so 2.30am simply never occurs.
- The mapping at 2.30am on November 4th 2012 is ambiguous: it happens once before the clocks go back at 3am, and once afterwards. The offset is either -7 or -8 hours, depending on which occurrence you mean.
When mapping a local time to "global" time, this is something you should really think about. Most APIs obscure the issue, but one of the purposes of Noda Time is to force developers to think about issues which they should be aware of. This one is particularly insidious in that it’s the kind of problem which is much more likely to arise when you’re asleep than during daylight hours – so it’s unlikely to be found during manual testing. (Ditto the day of week – most time zones have daylight saving transitions early on a Sunday morning.)
So, Noda Time exposes four ways of mapping a LocalDateTime and DateTimeZone to a ZonedDateTime:
- Exact: if there’s a single mapping, return it. Otherwise, throw an exception.
- Earlier: if there’s a single mapping, return it. If there’s more than one, return the earlier one. If the time is skipped, throw an exception.
- Later: if there’s a single mapping, return it. If there’s more than one, return the later one. If the time is skipped, throw an exception.
- All information: find out all the information relevant to mapping the given local value – how many matches there are, what they would be, what the time zone information is for each mapping, etc. The caller can then do what they want.
Options available
The question is how we expose these operations. Let’s look at some options, then discuss the pros and cons.
Option 1: methods on LocalDateTime
A lot of Noda Time is designed to be "fluent" so it makes a certain amount of sense to be able to take a LocalDateTime, perform some arithmetic on it, then convert it to a ZonedDateTime, then (say) format it. So we could have something like:
- var zoned = local.InZone(zone); // implicitly exact
- var zoned = local.InZoneOrEarlier(zone);
- var zoned = local.InZoneOrLater(zone);
- var mapping = local.MapToZone(zone);
Option 2: methods on DateTimeZone
All the calculations involved are really about the time zone – the local date/time value is just a simple value as far as most of this is concerned. So we can put the methods on DateTimeZone instead:
- var zoned = zone.AtExactly(local);
- var zoned = zone.AtEarlier(local);
- var zoned = zone.AtLater(local);
- var mapping = zone.MapLocalDateTime(local);
Option 3: methods (or constructors) on ZonedDateTime
Maybe we consider the two inputs to be fairly equal, but the result type is more important:
- var zoned = ZonedDateTime.FromLocal(zone, local);
- var zoned = ZonedDateTime.FromLocalOrEarlier(zone, local);
- var zoned = ZonedDateTime.FromLocalOrLater(zone, local);
- var mapping = ZoneLocalMapping.FromLocal(local)
(I’m not terribly happy about the names here; there could be better ones of course.)
Variation a: any of the above options, but with an enum for ambiguity resolution
We don’t really need four methods on any of these APIs; the first three only differ by how they handle ambiguity (the situation where a particular local date/time occurs twice). We could use an enum to represent that choice instead:
- var zoned = local.InZone(zone, ZoneAmbiguityResolver.Error);
- var zoned = local.InZone(zone, ZoneAmbiguityResolver.Earlier);
- var zoned = local.InZone(zone, ZoneAmbiguityResolver.Later);
(Or a "smart enum" with behaviour, if we wanted. A normal class type with methods etc, but only a restricted set of valid values.)
Variation b: always go via the mapping result
Given that we already have the idea of getting the full mapping results, we can reduce the API to just one method to return the mapping information, and then put extra methods on that:
- var zoned = local.MapInZone(zone).SingleMatch;
- var zoned = local.MapInZone(zone).SingleOrEarlier;
- var zoned = local.MapInZone(zone).SingleOrLater;
(Again, the names aren’t fixed in stone, and the second part could be methods instead of properties if we wanted.)
Variation c: return a sequence of results
If we return a sequence with 0, 1 or 2 ZonedDateTime values, the user can just use LINQ to get the one they want. Again, this can apply wherever we decide to put the method:
- var zoned = zone.At(local).Single();
- var zoned = zone.At(local).First();
- var zoned = zone.At(local).Last();
So, it looks like we effectively have two mostly-orthogonal decisions here:
- Where to "start" the conversion – the target type for the method call
- How to represent the multiple options
We’ll consider them separately.
Regarding the "source" type
To start with, I’ll reveal my bias: the existing implementation is option 2 (four methods on DateTimeZone). This was after a small amount of debate on the Noda Time mailing list, and this was the most relevant bit of the discussion:
Me (before going with the current implementation):
It feels a little odd to me to use the zone as the principal class here – just in terms of usability. It makes total sense in terms of the logic, but I tend to think of having a LocalDateTime first, and then converting that to use a particular zone – it’s not an operation which feels like it acts on the zone itself.
David N:
I actually feel the opposite: asking a DateTimeZone how a particular LocalDateTime would be represented in that zone feels natural, while asking the LocalDateTime how it would be represented in a zone feels odd. The zone is a first-class entity, with identity and behavior; the LocalDateTime is just a set of values. Why would the LocalDateTime be expected to know how it is represented in a particular zone?
Even though I replied to that in a "maybe" kind of tone, the argument basically convinced me. The trouble is, a colleague was then surprised when he read the documentation around calendar arithmetic and conversions. Surprising users is pretty much a cardinal sin when it comes to API design – and although in this case it was the relatively harmless sort of surprise ("I can’t find the member I want in A; oh, it turns out it’s in B") rather than a behavioural surprise ("I thought it would do X, but instead it does Y") it’s still bad news. I should reveal my colleague’s bias too – he has experience of Joda Time, where the relevant call is LocalDateTime.toDateTime(DateTimeZone). (There are calls in DateTimeZone itself, but they’re lower-level.)
We’ve discussed this a fair amount (leading to this blog post) and so far we’ve concluded that it really depends on how you think about time zones. As a Noda Time user, would you consider them to be rich objects with complex behaviour, or would you think of them as mere "tokens" to be passed around and used without much direct interaction? The two ways of viewing the type aren’t necessarily in conflict – I’ve deliberately designed CalendarSystem to hide its (fairly ugly) innards. There are a few public instance members, but most are internal. But what about time zones?
There’s an argument to be made for educating Noda Time users to think about time zones as more complex beasts than just tokens, and I’m happy to do that in other areas (such as choosing which type to use in the first place) but here it feels like it’s one step too far. On the other hand, I don’t want to stifle users who are thinking of DateTimeZone in that way. In the mailing list thread, David also expressed a dislike for the approach of including functionality in multiple places – and to a certain extent I agree (one of the things I dislike about its API is that it lets you do just about anything with anything)… but in this case it feels like it’s justified.
Regardless of how you’re thinking about DateTimeZone, it’s more likely that you’re going to want to use a LocalDateTime value which is the result of some other expression, and then apply some "constant" zone to it, then potentially keep going. If you think about a LINQ-style pipeline of operations, the part that varies in the conversion is much more likely to be the LocalDateTime than the time zone. As such, a method on LocalDateTime allows for a more fluent calling style:
.PlusMonths(1)
.InZone(LondonTimeZone);
versus:
Or to keep the code order the same as the execution order:
var zoned = LondonTimeZone.AtExactly(local);
Obviously the effects become more noticeable the more operations you perform. Personally I’m happy with either the first or third approach – although it’s worth being aware that either of the first two have the advantage of being one expression, and therefore easy to use when assigning a static readonly field or something similar.
I’m reasonably happy with having one method on each type, or possibly two (MapLocalDateTime and At*) on DateTimeZone and one (just InZone) on LocalDateTime. I really don’t like the idea of having four methods on DateTimeZone and three methods on LocalDateTime. So, let’s consider the different variations which cut down the number of methods required.
Expressing "exactly," "earlier," and "later" in one method
This is essentially a discussion of the "variations" above. Just to recap, the possibilities we’ve come up with are:
- Add another parameter to the method to indicate how to handle ambiguities (or impossibilities) – just return a ZonedDateTime
- Return a value of a different type (e.g. ZoneLocalMapping) which can be used to get at all the information you could want
- Return a sequence of possible ZonedDateTime values, expecting the caller to probably use LINQ’s First/Last/Single/FirstOrDefault etc to get at the value they want
The last of these is the only one which gives an easy way of handling the extreme corner case of a local time occurring more than twice – for example, a time zone which goes back one hour at 2am (to 1am) and then goes back another two hours at 3am. I think it’s reasonable to dismiss this corner case; however mad time zones can be, I haven’t seen anything quite that crazy yet.
At the time of the original discussion, Noda Time was targeting .NET 2.0, which was one reason for not going with the final option here – we couldn’t guarantee that LINQ would be available. Now, Noda Time is targeting .NET 3.5 in order to use TimeZoneInfo, but it still doesn’t feel like an ideal fit:
- Returning a sequence doesn’t give information about (say) the two zone intervals "surrounding" a gap
- A sequence may be surprising to users who expect just a single value
- The exceptions thrown by First, Single etc when their expectations aren’t met are very domain-neutral; we can give more information
- FirstOrDefault will return the default value for ZonedDateTime in the case of ambiguity. That would be unfortunate, as ZonedDateTime is a value type, and its default value is actually somewhat unhelpful. (It has a null calendar system, effectively. There’s not a lot we can do about this, but that’s a post for another day.) We could make it a sequence of Nullable<ZonedDateTime> and guarantee that any values in it are actually non-null, but that’s really straining things.
Putting these together, there are enough negative points to this idea that I’m happy to rule it out. But what about the first two?
The first has the advantage that the caller only needs to make a single method call, effectively passing in a "magic token" (the ambiguity resolver) which they don’t really need to understand. On the other hand, if they want more information, they’ll have to call a different method – and I’m not really sure we want to encourage too much of this "magic token" behaviour.
The second has three disadvantages, all fairly slight:
- The user may initially expect the result of a method mapping a LocalDateTime to a ZonedDateTime to be a ZonedDateTime… what’s this extra intermediate result doing? This is "only" a matter of user education, and it’s pretty discoverable. It’s an extra concept the user needs to understand, but it’s a good concept to understand.
- Calling two methods or a method and a property (e.g. zone.MapLocalDateTime(localDateTime).Earlier) may end up being slightly more long-winded than a single method call. I can’t get excited about this.
- We have to allocate an extra object for the mapping, even when we know it’s unique. Usually, this object will become eligible for garbage collection immediately. We could make it a struct, but I don’t think it’s a natural value type – I’d rather trust that allocating objects in gen0 is pretty cheap.
With the second method, we can replace all the existing methods in DateTimeZone with a single one (or rather, just remove the AtXyz methods, leaving MapLocalDateTime). We can then create pleasantly-named methods on ZoneLocalMapping (which isn’t quite right for this purpose at the moment).
Conclusion
This has been an interesting thought experiment for me, and it’s suggested some changes I will be applying before v1. We’ll see how they pan out. If you want to follow them, look for relevant source code changes.
The important points I’ve been thinking about are:
- What would a new user expect to be available? If they haven’t read any documentation, what are they likely to try?
- What should the user know about? Where there are important decisions to make, how can we provide guidance?
- What would an experienced user (who is already thinking about the Noda Time concepts in the way that we want to encourage) expect to be available?
- Where does the balance lie between providing a "too crowded" API (with lots of different ways of achieving the same thing) and a "sparse" API (where there’s always one way of achieving a goal, but it may not be the most convenient one for your situation)
- How does our choice fit in with other technologies? For example, the final "variation" seems like it plays nicely with LINQ at first – but a few subtleties make it a worse fit than it might seem.
- How does this affect performance? (Performance is important in Noda Time – but there would have to be a significant performance problem for me to deviate from an elegant solution.)
So, any other thoughts? Did we miss some options? What other factors should we have taken into consideration?
Jon Skeet's coding blog 
What about extensions methods? optional parameters? They can give you more options.
LikeLike
@Omer Mor: Given that we’re targeting .NET 3.5 we definitely don’t want optional parameters. I’m trying to avoid extension methods at the moment, in case we want to backport to 2.0 in the future. To be honest, as I’m in control of all the types here, I don’t think extension methods would really help anyway.
LikeLiked by 1 person
var zoned = LondonTimeZone.AtExactly(parseResult.Value.PlusMonths(1));
That line of code struck me because I’m not very familiar with the time zone names, so I wouldn’t know that an object called LondonTimeZone exists.
With other libraries, as a naive user I tend to rely heavily on enumerations to get me more familiar with the domain. When I’m not familiar with something (time zone names in this case), I would tend to start with what I do know and then work out from there.
As a user I would prefer to lay anchor around local time (or whatever it is) and then branch out from there into unknown waters (using different time zones).
Just my preference though.
LikeLike
I once learned a nice API design concept from Philip Laureano:
Keep your the public surface of your classes to the bare minimum (core logic only), and then add extension methods that expose richer options/API.
So in the case of Noda you can add a LocalDateTime.MapToZone method with all the various options, and then sprinkle extension methods on LocalDateTime & DateTimeZone for all the various usages you could think of.
LikeLike
@Ryan: In fact there *isn’t* normally a member called “LondonTimeZone” – that was just an example of where you might have a static readonly field called LondonTimeZone of type DateTimeZone.
There’s no enumeration of time zones, in the same way that there’s no enumeration of cultures – in the case of time zones, you’d do something like:
private static readonly DateTimeZone LondonTimeZone = DateTimeZone.ForId(“Europe/London”);
LikeLike
@Omer: As I mentioned before, I’m trying not to buy into extension methods *just* yet. There are also times where they’re somewhat ugly.
(If I did this, the core method would definitely be on DateTimeZone, as that’s the most logical place for it.)
Extension methods are great, but they can definitely be overused. The idea of the core functionality vs “helpers” is an interesting one (and one I’ve considered in other contexts) but it feels like the kind of thing that appeals more to advanced developers than “average” ones… and I’m definitely trying to avoid Noda Time being seen as only suitable for advanced users.
LikeLike
re: extensions methods – here’s the discussion I had with Philip Laureano about the subject:
http://plaureano.blogspot.com/2009/04/introducing-hiro-worlds-fastest-ioc.html?showComment=1239190140000#c5305200417711381937
LikeLike
It seems to me that the entire discussion about resolving ambiguities with “exactly”, “earlier”, or “later” is a discussion about exception handling, just like the case when the LocalDateTime cannot be converted at all.
I say this because:
1. It is rare
2. The LocalDateTime does not have enough information to resolve the ambiguity
While it seems like a good solution to either let the caller hard code a resolution with a resolver or return multiple values and let the caller deal with the problem, this is just kicking the empty can down the road and does not solve the problem.
I as a caller I’m no smarter than you, because if the local value cannot be unambiguously converted, then it cannot be unambiguously converted unless I have some extra piece of information available. Rather than hard code this extra piece of information into the code or carry it in a separate variable somewhere, if the occasional exception is cause for concern for me I would rather work in UTC in which such exceptions cannot occur.
So from a caller perspective, convert if you can do it unambiguously or throw an exception.
LikeLike
@Ferenc: I disagree. I think in plenty of cases it’s fine to say “If it’s ambiguous, always use the earlier version” or “If it’s ambiguous, always use the later version” – if that’s *not* the case, then by all means call the AtExactly version.
Now for the *gap* case, where the caller has provided a local time which *simply does not exist* in the given time zone, an exception is almost always appropriate, so that’s what will happen unless you just ask for the information and work it out from there.
You shouldn’t decide whether or not to work in UTC based on this problem though – you should decide that based on what data you’ve actually got. If a user has provided you with a local date/time and a time zone, then you *have* to perform the conversion. If you *don’t* have a local date/time and a time zone to start with, what are you going to convert? Making this kind of decision explicit is one of the goals of Noda Time – so people really think about what values they’ve got.
LikeLiked by 1 person
Quoted in a Skeet post! Cross that off my bucket list :)
Awesome dissection of the complexities of API design. This is a subject of particular interest to me, and I think it is worth taking the time to provide the consumer with the best experience possible.
I stand by my original statement: a zone has identity and behavior, while a LocalDateTime is a set of values. However, discoverability is a very important factor, and if users are going to look for this conversion on LocalDateTime first, that has to be taken into account. I also am still generally against having multiple ways to do the exact same thing; but if users are approaching the API from significantly different perspectives, maybe the best thing for usability is to cater to both perspectives. Personally I tend to think about all such conversions in the context of the zone; but that may be because I have experience doing time conversions in the context of a global financial services company, and I have studied the Olson time zone database and zone representation format. All users will not necessarily have that background. And I can see potential advantages in a fluent style that could be used with the LocalDateTime methods.
So while I don’t know that I would make the same decisions, I don’t think I can fault you for taking this approach.
I am not sold on ZoneLocalMapping as the default return value. Most of the time the consumer will know ahead of time which strategy they way to use to deal with ambiguities; they don’t need to return all of the information first before they decide. I think there needs to be a way for the consumer to express their desired strategy up front. I agree with you that encouraging “magic token” behavior is not a good idea; so I would go back to the AtExactly, AtEarlier, AtLater methods. I know you intentionally eliminated that option because you are concerned about the number of methods required, but I’m not really sure why; the number of methods accurately reflects the number of different behaviors, and it is hardly excessive.
I agree with you that extension methods are not the answer. Either the functionality belongs on a type, or it doesn’t. If it does, put it on the type; if it doesn’t, don’t. Extension methods just obfuscate the problem and make discoverability more difficult. Omer: the discussion you reference is specifically talking about extending interfaces, but that is not the case here.
LikeLike
@David: Thanks for the continued decisions. I haven’t committed anything yet – there’s still time to convince me :)
The previous approach (AtExactly/AtEarlier/AtLater) is still my “second favourite choice” but the more I’ve thought about the intermediate value approach, the more I’ve liked it. It feels more logical – there really *is* an intermediate result.
My current unsubmitted code uses your idea of a LINQ-like API, so on the “new” version of ZoneLocalMapping, there’s Single(), First() and Last(). Can’t decide whether this is too “cute” or may be misleadingly too much like LINQ. *sigh*
Oh, and one more thing – I’ve been considering turning ZoneLocalMapping into a nested type called LocalMapping within DateTimeZone. Thoughts on that? (We can continue on the mailing list, of course…)
LikeLike
At a high level, I think I disagree with the way you’ve described the available options. In most uses, I think I want .ToZonedDateTime() to return a single time no matter what. I can pass in information on how to handle ambiguities, but getting a single ZonedDateTime shouldn’t require multiple code paths.
If I’m running a task at 2:30AM every month and the user happens to be travelling in a time zone where 2:30 doesn’t exist, it’s fine if I run at 3:30. Or if a user makes a blog post at 2:30 and their time zone says that 2:30 exists twice today, I just want to use one or the other (the later one is usually best). It’s OK to make me as the developer think about this stuff, but once I’ve thought about it I want a single code path to use it.
It’s reasonable to have me call LocalTime.ToZonedTimeZone(resolver) in these cases. But I really hate the idea of having to deal with try/catch or any kind of if-branch logic here. That flies in the face of the whole fluent interface idea, since I need to repeat the whole sequence again in the catch block. And, of course, most people won’t. There’s a strong tendency to just re-throw rather than do something reasonable.
Given that increased set of options, I think you want an enum or more complex resolver class in a single LocalTime.ToZonedDateTime(resolver) call rather than multiple methods.
LikeLike
@DWoof: In any one situation, I’d expect you to only call one method. That’s already the case for handling ambiguities – for your blog post version you’d just use AtLater() or whatever. No conditional code on your part.
The situation we *don’t* handle without “extra help” is the impossibility situation – the “running a task at 2:30am every month”. Should it run at 3:30? Or should it run at 1:59:59? Or maybe at 3:00? Of course you can easily write your own single method to be called from multiple places to express your single set of business rules – but I don’t expect those rules to be particularly universal.
We may well want to streamline this when we’ve got user feedback as to which requirements *are* actually common… but at the moment it would be pure speculation.
LikeLike
I also think that in the typical case you’ll want one answer no matter what; that use case should work without too many issues.
So, like DWoof says, you’ll want to at least allow a scheduling resolution in which even impossible cases resolve to *something*.
Separately; the “multiple options” return value isn’t actually necessarily distinct from the Linq/IEnumerable approach.
After all, you can return some typed value which simply has methods First/Last/Single etc. that throw “meaningful” exceptions as you want and simultaneously implement IEnumerable – members will resolve before Linq’s extension methods.
As an API consumer, I’d prefer a small surface over many (confusing) options, so I’d vote to go for the most logical and stick to that if all that cleanliness costs is just occasional pair of brackets. My vote would thus be for a single method; no overloads or enum magic. The consumer can choose which behavior to use by a further method on the return value; here you could have loads of options such as first/last/single/fuzzy/enumerable as needed.
Also, I think that a value type is quite reasonable: after all, your option-object is fully defined by its components. Any 13:37 in EST is equivalent to anothoer 13:37 in EST… To use a struct vs. a class sounds to me mostly like a performance and simplicity issue. If you want built-in reasonable Equals and a struct isn’t too limiting it’s a simple and usually performant solution, but if you’re afraid of multithreading issues, the various perf problems they can cause, or need more control over the semantics, you’ll need a class.
LikeLike
I accidentally (honestly!) deleted a comment from Xinju Chi just now. Here it is:
> With due respect, I think that the concept of ZonedDateTime is flawed because of its ambiguity and impossibility you demonstrated. You should discard it in Noda Time.
I *completely* dispute that the concept is flawed. The ZonedDateTime concept makes perfect sense, and *it* can neither be ambiguous nor impossible. It’s just that not every local date/time value has a corresponding value in a time zone.
That’s not a matter an API can discard – it’s the reality of time zones.
LikeLike
Couldn’t you solve this using Monads?
Instead of returning a DateTime object could you return a SuperDateTime object.
SuperDateTime.Value would always return a DateTime, however SuperDateTime could have other properties that would alert the user to possible problems, like SuperDateTime.IsAmbiguous etc…
You could keep all possible DateTimes in the SuperDateTime.Values property which would be an IEnumerable which users can then query using LINQ (i.e. .Single(), .First(), .Last()
LikeLike
@BenV: So we’d have LocalDateTime (no zone information), ZonedDateTime (a precise momement in time, in a time zone) and SuperDateTime which would be a half-way house? I think that’s pretty similar to the ZoneLocalMapping I’ve described in the post – but you’ve just added an extra Value property which would have to always give a value, presumably taking some arbitrary but documented decision in the case of both ambiguity and impossibility.
LikeLike
I’ve not used NodaTime but looking at the options you’ve given I would stay away from LINQ – returning a list of values would be confusing to me.
I personally like the idea of returning an intermediate class like you suggested (ZoneLocalMapping), but for those that don’t care about how it should deal with errors you could have the ZoneLocalMapping be implicitly convertible to ZonedDateTime, so the properties are there for more advanced users but beginners wont be troubled with an issue that may never occur.
LikeLike
@Sam: I wouldn’t want to use an implicit conversion, as there’s no really sensible default behaviour. One of the points of Noda Time is to *force* users to be aware of potential issues – make them care about inherent complexity.
LikeLike
Personally Option 1 seems more intuitive on first sight.
Actually the SuperDateTime was only a (silly) name ;)
Using the Match type as a precedent in the System.Text.RegularExpressions namespace couldn’t you load LocalDateTime with a few more properties?
Such as:
LocalDateTime.IsAmbigous Match.Success
LocalDateTime.Value Match.Value
LocalDateTIme.Values Match.Groups
You may be able to remove the InZoneOrEarlier and InZoneOrLater methods this way…
The user should first check the value for IsAmbigous. If it is not ambigous then .Value has a valid value, otherwise they have to decide which of the .Values to use.
LikeLike
@BenV: As David raised in the email discussion, this isn’t really something that LocalDateTime should have knowledge of – and you don’t want to have to *explicitly* ask whether something is ambiguous, IMO… you want to express the action to take *if* it’s ambiguous.
IsAmbiguous certainly can’t be a *property*, because it’s not that a LocalDateTime is inherently ambiguous – the ambiguity only occurs when you try to map it in a particular time zone.
LikeLike
@Ben: Alternatively, to continue your regex analogy:
LocalDateTime => String
DateTimeZone => Regex
ZoneLocalMapping => Match
So I *could* provide some convenience methods on DateTimeZone which are equivalent to Regex.IsMatch, but which just go via a ZoneLocalMapping… but I’m hoping to keep the API relatively small to start with.
LikeLike
Jon, you are certainly faced with an interesting problem.
Thanks for correcting my analogy. LocalDateTime should remain as simple as possible, and the ZoneLocalMapping could contain some extra information about ambiguity.
To recap some of your requirements for the API you are looking for:
1 – A small footprint API
2 – An intuitive API
3 – An API that *forces* users to be aware of potential issues
I think there is an inherent conflict between 2 and 3.
Point 2 would apply to new comers that are not totally aware of the ambiguities of time zone converstions. They would probably favour option 1 and assume the InZone method to always return a valid answer.
Point 3 would apply to users that were familiar with time zone conversions and would favour option 2.
Then again, if they are going to use Noda Time in the first instance, it is probably because they are already aware of the perils of time conversion, so favouring point 3 might be the way to go.
Ultimately it becomes a question of whether you want the API to be “reactive” or “proactive”. I will follow the rest of the conversation with interest, and look forward to see which decision you make and why.
LikeLike
@Ben: I expect Noda Time users are probably aware that there *are* perils, but may not know what they are yet :)
As for where I go with this – I’m currently still on the fence :(
But thanks for bringing up the regex analogy – it’s a really useful one, I think. Given some of the other comments, it also does sound like we need some call that guarantees it’ll give *some* answer, even if it’s one with a different LocalDateTime to the one provided…
LikeLike
Exceptions are a pretty inefficient way to educate users – what about using the “TrySomething” idiom for exact conversions?
var zoned;
if (local.TryInZoneExactly(zone, zoned))
Console.WriteLine(“It’s {0} in {1}!”, zoned, zone);
else
Console.WriteLine(“{0} didn’t occur in time zone {1}”, local, zone);
This is really awkward, but that’s part of the point. It’s hard to think of many scenarios in which callers will really want exact conversion. (Am I wrong about that?) This is so ungainly it might induce them to try to understand the other options.
LikeLike
@Jeff: I never suggested using exceptions as a primary way to educate users. I’m suggesting using several different methods to make it clear that there are different options around this. If they then ask for a conversion which is impossible, I think an exception is a reasonable response.
And yes, I think you’re wrong about not really wanting a correct conversion. It doesn’t necessarily have to be unambiguous, but it should accurately map the original value. If I asked for a conversion from a LocalDateTime to a ZonedDateTime and the returned value *didn’t map back to the same LocalDateTime* I’d be pretty confused, if I hadn’t *explicitly* requested some sort of lenient behaviour.
I’m leaning back towards the idea of having a set of “resolvers” – in some way which gives easy access to common behaviours, but also provides simple flexibility for custom versions.
LikeLike
Gah, good ol’ foot-in-mouth disease.
It’s hard to think of scenarios where a caller wouldn’t want to fall back to one of the alternatives if an exact conversion isn’t possible.
I like the “resolvers” approach too, since it signals to callers that there’s something they need to understand.
LikeLike
@Jeff: New thought:
// Very strict: must be unambiguous
DateTimeZone.AtStrict(LocalDateTime)
// Very lenient: gives back a result whatever
// (Details TBD)
DateTimeZone.AtLenient(LocalDateTime)
// Custom
DateTimeZone.At(LocalDateTime, Resolver)
where Resolver (or ZoneMappingResolver or something similar) would be an interface, with several implementations in Resolvers.Strict, Resolvers.EarlyIfAmbiguous etc.
(And also MapLocalDateTime to get the “raw” ZoneLocalMapping.)
My guess is that this would satisfy you, but not David Nelson. I suspect that it’s going to be impossible to make everyone happy here :)
LikeLike
Among the variations, I prefer b i.e. the custom class with three clearly labeled methods, and a way to enumerate all results.
I’m not sure if this class should *be* an `IEnumerable` or if it should *have* `IEnumerable`.
My main issue with this variant is, that people using `SingleOrEarlier` / `SingleOrLater` might have the expectation that they never throw an exception, after all they made a choice which one they prefer in that case.
Perhaps they should be forced to choose on both exceptional cases explicitly by specifying an AmbiguityResolver and a NonExistanceResolver.
—
Btw I still believe that `Duration` should have methods or properties to easily convert from/to units of time with constant length. i.e. Seconds, Minutes, Hours and StandardDays.
The parameter type of these methods is also an interesting choice. Should it be restricted to integers?
At least for `FromSeconds`/`TotalSeconds` I think `Decimal` would be the best type, because it can exactly represent any `Duration`.
—
Another minor issue is that `Local` means “unspecified timezone”. I’d understand `Local` as using the timezone of my computer(in the case of web apps the client’s timezone).
Unfortunately I don’t have any brilliant naming ideas either, `UnspecifiedDateTime` is a bad name too.
—
I’d also like an easy way to convert between the build in .net types, and the node types(at least for types where this makes sense). This probably should be in a separate static helper class, to avoid polluting the Noda types.
LikeLike
@CodeInChaos: Leaving aside the time zone info bits:
> Btw I still believe that `Duration` should have methods or properties to easily convert from/to units of time with constant length. i.e. Seconds, Minutes, Hours and StandardDays.
It does – see the most recent API docs. I’ve kept to “long” for the types of these though, as I want a duration to be regarded as an *integer* number of ticks.
> Another minor issue is that `Local` means “unspecified timezone”. I’d understand `Local` as using the timezone of my computer(in the case of web apps the client’s timezone).
I think this is due to the poor choice of naming in .NET, to be honest – I’m happy with the name Local, but I’m not happy with TimeZoneInfo.Local etc.
> I’d also like an easy way to convert between the build in .net types, and the node types(at least for types where this makes sense).
Already done, e.g. LocalDateTime.FromDateTime, Duration.FromTimeSpan/ToTimeSpan etc.
LikeLike
Although converting from local to zoned time is the most common, consideration should be given for going from one zoned time to another.
Although there aren’t the difficulties you describe above, it would be beneficial if the API were consistent.
Taking your example:
> var zoned = local.InZone(zone, ZoneAmbiguityResolver.Error);
What happens if “local” gets changed from being of type LocalDateTime to being of type ZonedDateTime? In many ways it would be nice if the API didn’t have to change too.
However, if we look at the other direction (code which originally converts from one zoned time to another zoned time, but gets updated to convert from local time to zoned time), we probably do want the code to break, because we want the developer to think about the 0/1/2 matches issue.
Not sure what the solution is, but my point is that I don’t think local to zoned conversion can be considered independently of zoned to zoned (or even zoned to local) conversion.
LikeLike
@Silverhaide: You raise a good point – I don’t think I’ve got a ZonedDateTime -> ZonedDateTime conversion, but I should do.
However, I would explicitly *not* want it to look like the LocalDateTime to ZonedDateTime conversion. One conversion involves a decision, the other doesn’t. You shouldn’t assume that changing a variable between the two types will keep the same behaviour at all – indeed, the types of arithmetic you can perform on them differ very significantly.
I will almost certainly name the “zone changing” version WithZone, which fits in with what I believe I’ve done elsewhere. (I need to document my conventions for prefixes such as At/In/With/From/To, but hopefully I’ve been reasonably consistent…)
LikeLike
@Silverhalide: Ah, it turns out I already have ZonedDateTime.WithZone :)
LikeLike
I *strongly* dislike your current `TotalXXX` properties (except `TotalTicks`). IMO the truncating behavior violates the principle of least surprise. They have the same name as their .net cousins, but different behavior. Worst of all, they will still compile in `double hours = duration.TotalHours`, but give a different result. I’d rename them to `FullXXX` e.g. `FullHours`.
The `StandardDays` property doesn’t even fit into the same naming convention as the rest of them.
Then I’d add `decimal TickInMinutes = 0.0000001m` to `NodaConstants`. And finally a `decimal TotalSeconds{get{return TickInMinutes * TotalTicks; } }` to duration.
This implementation has two nice properties:
1. It can exactly represent each `Duration`
2. The number of displayed decimal digits fits the tick accuracy.
I’d also define `ToString` as `TotalSeconds.ToString()`.
——————————–
`public Duration(Interval interval)`
I don’t see the purpose of this constructor. It’s less readable than `interval.Duration` and offers no advantages over it.
I don’t see much purpose in
public Duration(Instant start, Instant end)
public Duration(long startTicks, long endTicks)
either. `end – start` is certainly more readable than `new Duration(start, end)`.
——————————–
At first I misunderstood what the properties in the `MillisecondsRemainder` family do. I expected it to represent the part of the `Duration` that’s below milliseconds. i.e. the remainder of truncating to milliseconds.
Unfortunately I can’t think of a better name for them.
——————————–
Another interesting question is how you want to deal with integer overflow. I’d consider throwing an exception, instead of using `unchecked` behavior.
I’d also consider disallowing `new Duration(long.MinValue)`, due to its strange behavior when negated.
——————————–
Coming from a physics background, I’d also like to be able to do time arithmetic, like `duration * 1.5`, but that’s relatively hard to implement correctly(there are precision issues interacting with double), so I’d wait for the V2 API for that.
LikeLike
@CodeInChaos: I’ll give TotalXXX more consideration, but I still don’t like the idea of returning hours.
StandardDays very deliberately doesn’t fit into the same naming convention:
– There’s nothing “greater” than days, hence no StandardDaysRemainder
– It’s StandardDays instead of Days, as a day can last different amounts of time.
For duration things… I think I agree about the Interval constructor, but possibly not about the others. I’ll think about them further.
In terms of integer overflow: the whole of Noda Time is compiled in a checked context, other than places we know it’s safe. So you *will* get an exception, likewise for negation.
I’d rather steer clear of floating point arithmetic as far as possible, to be honest. Will happily consider use cases though…
LikeLike
@CodeInChaos: I should have said before: blog post comments really aren’t the ideal place for discussing the whole of the API. I suggest you post to the Noda Time mailing list, which is more appropriate.
Comments on *this blog post* are appropriate here, of course.
LikeLike
///
/// This is used as an optimization. If the time zone has not transitions but returns true
/// for this then the behavior will be correct but the system will have to do extra work. However
/// if the time zone has transitions and this returns false then the transitions will never
/// be examined.
///
public bool IsFixed { get { return isFixed; } }
Did you mix up `true` and `false` here? And why can’t this be determined by checking if `MinOffsetTicks==MaxOffsetTicks`?
————–
`DateTimeZone.GetSystemDefault()` being able to return `null` stinks. I expect most applications to break in that case, with a useless exception. Can’t you return some kind of dummy `DateTimeZone` that forwards conversion operations to windows, or fallback to UTC, or a constant offset timezone with the same offset as `DateTime.Now – DateTime.UtcNow`?
If not, I’d got with throwing a descriptive exception, and add another method for advanced users who want to handle that case. I expect >90% of applications to not care, and end up with a `NullReferenceExceptions`, or if they are luck with a `ArgumentNullException(“zone”)`.
Converting an `Instant` to the users local timezone is a common problem(for users like me it’s pretty much the only time where something other than `Instant`/`Duration` is used), and throwing such a difficult error handling problem in its way, which most users won’t solve in a reasonable way. That’s a kind of problem I expect a time library to handle for me.
LikeLike
@CodeInChaos: As mentioned before, this blog post isn’t the right place to do a code review of the whole of Noda Time. *Please* take it to the mailing list. I’ll delete further comments from here.
(Don’t get me wrong, I definitely want to get all this feedback – preferably in separate mail threads for easier discussion – but this simply isn’t the place.)
LikeLike