I took a closer look and did find an easier way. But I also ran into the curiosity of how things behave when we have an unspecified kind on a date. Let's take a look.
Simplifying Code
The code we start with isn't that complex.
Original Class |
This uses the Solar Calculator package from NuGet. The second line (in each method) creates the SolarTimes object that we need by passing in the date and latitude/longitude. The other 2 lines of code converts that date/time to a specific timezone.
It turns out that we don't need a specific timezone for our application. Instead we can use the local time. This makes the code much simpler. Here's the updated code:
Updated Class |
Instead of converting the "Sunset" and "Sunrise" properties to a particular timezone, we just take the values as-is. And this gives us the output that we expect:
This code works because the "Sunset" property represents local time (sort of). The DateTime structure has a "Kind" property. We'll need to explore this a little further.
Note: I also changed the name of the class. The package I used was also called "SolarCalculator", so there could have been some nasty side-effects. It makes sense to append "SunsetProvider" since that is how this is used in the rest of the application.
DateTimeKind
The "Kind" property of a DateTime is an enum (DateTimeKind) that has these values (from the MSDN documentation):
This means that we can mark a DateTime as being "Local" or "Utc". That should make things really easy.
So, let's see what "Sunset" is:
Oops, it looks like the "Kind" is "Unspecified". But if we look at the value itself, we can see that it represents the local time for sunset.
Changing the DateTimeKind
Even though this has the right value, I feel like things would be better if we could change the "Unspecified" to "Local". And this is where we see some of the weirdness when we have an "Unspecified" kind.
DateTime has a "ToLocalTime" method (from MSDN documentation):
This looks pretty promising. After running this method, the "Kind" will be "Local". But this method assumes that the "Unspecified" time is UTC. So, if we use it, we end up with an unexpected value:
That's not what we want; sunset is not just after noon. And this value is completely wrong (since it now represents a local time).
Let's try again. DateTime also has a "ToUniversalTime" method (from MSDN documentation):
This method makes the opposite assumption. This method assumes that the "Unspecified" time is Local. So, if we use it, we end up with an unexpected output:
This is actually a bit closer. The value is correct. Yes, I know that sunset is not at 2:30 in the morning, but the Kind for this is "Utc". So the value is correct, even though the display is not what we want.
A Smelly Solution
The value is correct (2:38 a.m. UTC is 7:38 p.m. Local), so let's see what happens when we combine these methods:
Because the Kind is no longer "Unspecified" when we call "ToLocalTime", we get the output that we expect. Let's look at the progression:
- Sunset = 7:38:58 p.m. (Unspecified)
- ToUniversalTime() = 2:38:58 a.m. (UTC)
- ToLocalTime() = 7:38:58 p.m. (Local)
But this just smells bad to me. Luckily, there is another option.
Setting DateTimeKind Directly
Our other option is to set the DateTimeKind directly. Well, sort of directly. The "Kind" property is read-only, but we do have a method that we can use:
This allows us to do the following:
This returns a new DateTime object that has the Kind set to "Local". And it has the value that we want as well:
If we really want to avoid having an "Unspecified" date/time, we can set it this way. And this method is much less smelly than converting a time to UTC and then converting it back to Local.
Wrap Up
So we've seen that when we deal with DateTimeKind, things get interesting if the value is "Unspecified". The "ToLocalTime" method assumes that the value represented is UTC, so it does a conversion. The "ToUniversalTime" method assumes that the value represented is Local, so it does a conversion. If we're not careful, we can end up with an incorrect value.
Working with DateTime is not as easy as it looks -- and we're not even dealing with any crazy timezone rules here. For a deeper dive, be sure to check out Matt Johnson's material, including the Pluralsight course Date and Time Fundamentals. (And I'm sure Matt has some specifics to add to this discussion.)
I like to simplify code wherever possible. Since this application doesn't need to deal with specific time zones, we can remove that code and deal with the local time. And to figure this out, I got to explore the DateTime object a bit more and learn how to use the "Kind" property. Learning something new is always fun.
This approach isn't the right solution every time, but it works just fine here. Sometimes we do care about specific time zones, but that's not needed in our case. The updated code is easier to approach, and I prefer to keep things simple where practical.
Happy Coding!
Yep, that's the way to do it. :) Though ideally, the SolarTimes library should have set DateTimeKind.Local for you, since that's what it returns.
ReplyDeleteAnother way to handle this would be to change the methods of your ISunsetProvider interface to return DateTimeOffset instead of DateTime. In the implementation, you would return new DateTimeOffset(solarTimes.Sunset). The constructor will treat Unspecified as Local, and assign the correct offset.
The value there is that you could now use the ISunsetProvider in a web application. In web applications, Local isn't all that clear, since it means local to the server where the code is running, which is not necessarily the same as the user of the web site.
For local desktop applications, you're fine with DateTime.