Reviewing Options: Building on Experience of Other Developers

Yesterday, I took a look at a couple of options to fix a problem with a fluent API in an application: Fixing Problems with a (Specific) Fluent API. I got a couple of really good comments, so I'm going to explore things a little further.

Note: This probably won't make much sense unless you look at the previous article. To get an overview of the entire project, check here: Rewriting a Legacy Application.

Opinions of Other Developers
I elicited comments from other developers. As usual, this is a great chance to get other points of view and also learn about some tools or techniques that maybe I don't know about.

First, a comment from Mackenzie Zastrow:

This is actually the direction that I'm leaning. It's hard to argue about the utility (or lack of utility) in the custom types. I'm not totally attached the the fluent API; I put it in as more of an experiment. So, I wouldn't be sad to see it go.

But then I got a good comment from Cliff Chapman:

New to Me
I hadn't heard about Semantic Types. There is an article on CodePlex by Matt Perdeck that describes the concepts: Introducing Semantic Types in .NET. And he's also created a NuGet package with the base types so that you don't need to code it up yourself.

This is really interesting to me. I think it's probably too much for this particular application, but it will be good to explore the concepts a little more.

Let's take a quick run through the code, describe the general idea, and look at a simple implementation.

Original Custom Types
The custom types that we started with didn't have much functionality:

These types just hold a single string value. The advantage that we get from this is that we now have custom types that we can use for our extension methods. So instead of extending all strings, we can extend just these types.

But we also have a problem. There's nothing to prevent us from putting whatever string value we want in these objects. So even though we get a bit of safety in our extension methods, we have a lot of danger in the objects themselves.

If we create these objects with invalid strings, then our code would blow up further down the line.

Adding Validation
The primary idea behind Semantic Types is that we can have a class that contains a "string" value, but we can constrain that string value to make sure it conforms to a particular format or value.

For example, we can have an EmailAddress object that will only accept values that pass a regex check for email address formatting. In addition, the Semantic Types classes deal with the issues of equality, comparison, and even checking valid values without creating a custom object.

It's an interesting article; I'd recommend a read. Here's the link again: Introducing Semantic Types in .NET.

For my project, I decided I didn't want to go all the way with using the Semantic Types base classes (but I'll definitely keep this in mind for future projects). Instead, I just added some simple validation to the existing types.

The code for this is in the FluentAPI-Updated branch on GitHub. The custom types are in the "SunriseSunsetOrg.cs" file in the "HouseControl.Sunset" project. Direct link to the file: SunriseSunsetOrg.cs.

UTCTimeString
Let's start with the UTCTimeString object. Here's the updated code:

We've just added some parameter checks to the constructor. If the value is "null", then we throw an ArgumentNullException. Then we try to parse the string as a DateTime object. If the parse fails, we throw an ArgumentException. If it passes that parse, then we assign the value to our property.

ResponseContentString
The ResponseContentString object has been updated in a similar way:

This has the same "null" check, and then it tries to deserialize the string as a JSON object. If this fails, we throw an ArgumentException. If it does not fail, then we assign the value to our property.

Note: This will work with *any* JSON object; it's not looking for specific fields in the object (which we might want to do). But we get the general idea of how we can add validation here.

Testing Validation
The nice thing about these objects is that we can test the validation very easily. For this, there is a new "HouseControl.Sunset.Test" project. The "SunriseSunsetOrg.Test.cs" file contains our tests (direct link: SunriseSunsetOrg.Test.cs).

Here are the tests for the UTCTimeString class;

These are pretty straight-forward. The first test is the "success" state. We pass in a valid time string and check that it is reflected in the "Value" property of the object. (As a reminder, the "Value" property is a string, not a parsed DateTime.)

The second test checks a "null" parameter. And as we can see from the "ExpectedException" attribute, we expect that this will throw an ArgumentNullException.

And the third test checks an invalid string value. This expects to get an ArgumentException. When we set up "ExpectedException" like this, the test will *only* pass if that particular exception is thrown. If there is no exception (or a different exception is thrown), then the test will fail.

With these in place, we can see that all 3 of these pass:

Now, let's take a look at the tests for the ResponseContentString:

These are similar to the previous tests. They test a valid value, a "null" value, and an invalid value.

And we get similar results when we run the tests:

So we can see that our code is working as expected at this point.

[Update 1/29/2015: I just realized that the test names don't reflect what we're actually testing. This is left over from a previous iteration of the code which had an "IsValid" property that was checked by the tests (if you're curious, it's in this commit). I changed the code to throw an exception instead and updated the unit tests (but I forgot to update the names). Here are the better test names. These are in the FluentAPI-Updated branch:

The tests with valid parameters are now titled with "HasValue", and the tests with invalid parameters are now titled with "ThrowsException". This better reflects what's going on in the tests.]

Semantic Types
So this is very simple validation for the custom types. I didn't go with Semantic Types for this example. I'll be experimenting with it in other projects by using the base types provided in the NuGet package.

What makes me most interested in it is that it seems similar to what F# has built in with Units of Measure. This allows you to make a type distinction between an integer that holds a temperature and an integer that holds a distance in miles.

I'm very interested in functional programming and cool features offered by various functional languages (and I really should be spending more time with it). Several of the concepts jump out at me, and this is one of them.

Semantic Types isn't exactly the same thing as Units of Measure, but it does give us the custom types that are based on more primitive types but are still distinct from one another.

(And I know that the similarities between these two concepts breaks down pretty quickly, but it's something that came to mind as I was looking at this.)

Which Option?
After all of this, I'm still leaning toward Option #2 (which is to not use custom types and remove the fluent API). For this project, I think it is the right choice since we still don't get much utility from the custom types. It will be interesting to explore Semantic Types in other scenarios.

I still haven't merged back to the "master" branch yet. I'll probably be doing that in the next few days. If you have an opinion one way or the other, be sure to leave a comment.

I value opinions of other developers. We all have different experiences in different environments. It's really great when we can leverage that experience without having to fumble through things on our own.

Happy Coding!

Fixing Problems with a (Specific) Fluent API

I'm still working on my home automation project. Recently, I've been dealing with the code that gets data for sunrise and sunset times -- and it's been getting a lot better. But in my last update to the code, I put in some extension methods so that we code using a fluent API.

I've since thought about this, and based on the problems with the current implementation, it needs to be updated. There are 2 directions we can take with this, so I created some branches and coded up both of them. After further consideration, I'll make a decision on which one will be merged back into the master branch.

The code (and articles) for this project are collected here: Rewriting a Legacy Application. The code is on GitHub: jeremybytes/house-control. And we'll be looking at several branches here.

The Problem Code
The problem code is in the FluentAPI-Original branch (we're specifically looking at the "SunriseSunsetOrg.cs" file in the "HouseControl.Sunset" project). The main idea behind this code is that we have a set of extension methods on the "string" class:

If you're not familiar with extension methods, check out Quick Byte: Extension Methods.

We're paying attention to the first parameter here (the one with the "this" keyword). Because the "GetSunsetString" takes a string and returns a string, and the "GetLocalTime" takes a string and returns a DateTime, we can write code like this (where "cacheData" is a string):

I like this syntax (and I've always liked fluent APIs such as LINQ). And this code works fine in isolation. But if we step back and take a broader look at what we've done with this code, it starts to look a little less brilliant.

Specifically, the extension methods extend the "string" class. This means that the methods really should work with *any* string that we give them. But the reality is that they only work with very specific strings: a JSON object and a date/time string.

But because these are extension methods on "string", there's nothing to stop us from using them on other string objects:

This is really bad. If we have a string that contains "Hello", we can call "GetLocalTime" on that. And the code completion in Visual Studio is almost encouraging us to do that. But it would just result in an error.

Falling into the Pit of Success
I really don't like code that lets developers get themselves into trouble so easily. When I create a library, I really want the developers to fall into the pit of success. This means that I need to remove any dangers that I can. This code is too easy to use incorrectly.

And there are really 2 directions that we can go with this. First, we can create more specific types that we use for our extension methods. Instead of using "string", we would use a custom type that contains the JSON data or the date/time data as appropriate.

The other option is to remove the extension methods (and along with it the fluent API). We'll explore both of these options.

Option #1: An Updated Fluent API
If I decide that I really like the fluent API, then we can use the code that makes the API a bit safer. This code is in the FluentAPI-Updated branch.

The extension methods have been updated to work with custom types (instead of string):

We have 2 new types -- "ResponseContentString" and "UTCTimeString" -- which are designed to hold the JSON result of our service and a string that represents a UTC time, respectively.

Notice that "GetSunriseString" and "GetSunsetString" both take a "ResponseContentString" as a parameter and return a "UTCTimeString". Then we can use that "UTCTimeString" with the "GetLocalTime" method to return the local date/time.

The custom classes themselves are pretty simple. In fact, I just added them to the top of the same "SunriseSunsetOrg.cs" file.

These just have a single property to hold a string value. But because they are custom types, we can target our methods a bit better.

When we have these in place, we can still use the same fluent syntax:

But in order to do this, we've had to change a few of our variables. Notice that the "cacheData" field is now a "ResponseContentString".

And if we look at the methods on this type, we see that they are a bit more focused:

So we see that we have "GetSunriseString" and "GetSunsetString" available to us (but not "GetLocalTime").

And if we tried to make a call on a "Hello" string (like we did above), we would not see any of our extension methods since they now operate on these custom types.

Note: As another alternative, we could move the extension methods so that they are part of the custom classes themselves. But before doing that, we need to think about whether it confuses the responsibility of each class. I could argue either way in this case. We'll stick with the separate extension methods here.

Advantages & Disadvantages
The biggest advantage to this approach is that we get to keep the fluent API. The biggest disadvantage is that we now have more custom types floating around our code.

So, we're making our code a bit more complex in order to make using the API a bit simpler. Like everything else we deal with when making decisions about our code, everything is a bit of give and take.

Now let's take a look at another option.

Option #2: Remove the Fluent API
Our other option is to get rid of the fluent API by removing the extension methods. This code is in the FluentAPI-Removed branch.

We're not removing the functionality that was in the extension methods, we're just turning them into normal methods:

These methods are now in the "SunriseSunsetOrg" class instead of the "SunriseSunsetOrgHelpers" class. And they are now just static methods. Notice that the "this" keywords have been removed from the parameters.

These have the same functionality, but we end up calling them a bit differently. Here's our calling method:

Notice that our fluent syntax is gone. Now we're calling methods with parameters and passing results as parameters to the next method. We could tighten up this code a little bit (by removing some of the intermediate variables), but it gets more difficult to read and debug.

For a look at why we may or may not want the intermediate variables, check out I Can Write That Method with One Line of Code.

Advantages  & Disadvantages
The biggest advantage to this approach is that we keep things simple. We don't add any new custom types (notice that the "cacheData" field is just a "string" like it was before).

The biggest disadvantage is that we lose the fluent syntax. Again, I like fluent APIs (where they make sense). And I'm okay with losing access to the fluent nature of the API if it's not appropriate here.

Which Would You Choose (and Why)?
I am going to merge one of these options back into the master branch of this project. So the question is, which one should I choose? And is there a good reason to choose one over the other?

The branches are all on GitHub:

I'm planning on leaving these branches in the repository even after I've merged the branch that I want. That way, the code will stay there to go along with this article.

So, check out the FluentAPI-Original branch to see the code in its problem state (with the extension methods on "string").

The FluentAPI-Updated branch has the code for Option #1 where we maintain the fluent API by adding custom types.

The FluentAPI-Removed branch has the code for Option #2 where we remove the extension methods and do not add any custom types.

[Update 01/28/2015: Based on the comments on this article, I've explored some options a little further: Reviewing Options: Building on the Experience of Other Developers.]

Wrap Up
I'll be considering the pros and cons of these options over the next few days. Neither one of these affect the operation of the application (in fact, the "problem" code is running just fine in production). But I'm using this application to experiment with different techniques. This lets me practice my skills so that I can more readily use them in other environments.

Coding practice is important, and it's something that many developers never do. When we go through exercises like this one, we think through techniques and approaches, and we find out what works well for us and what doesn't. That way, when we get into "crunch time" in a real application, we know exactly what approach will work best for us.

Happy Coding!

Separating Concerns in Methods

I'm a big fan of readable code. And I especially like it when there are clear pathways of functionality in code. Sometimes, things don't come out quite right the first time, so we need to work on it. This happened to me a couple weeks ago (First Tries are Sometimes Ugly). Fortunately, code is not carved in stone; we can go back and keep improving it.

The code that we're looking at is designed to get the sunset time from a service. Once we have this value, we create home automation events relative to that time -- for example, having lights come on 30 minutes before sunset or 1 hour after sunset.

All of this code (and the articles) are collected here: Rewriting a Legacy Application. The completed code for this article is in commit 9e3ad90f33 of the GitHub project: jeremybytes/house-control.

A Bit of a Mess
It's good to get help from folks who have experience with stuff that you don't. I was able to tap into Matt Johnson to help me fix the date/time parsing issues that I had (details: Code Gets Better by Sharing Experience and Expertise). But that was just one part that needed fixing.

Here's how we last left the "GetSunset" method which is part of the "SunriseSunsetOrg" project:

This has the date/time parsing updates in it, so it isn't as ugly as it was, but it still needs some help.

Here are the items that need work (as noted in the prior article):

• We probably want to split out functionality into separate methods (service call, JSON parsing, date/time manipulation).
• The Lat/Long values need to be a bit more dynamic (probably coming from configuration).
• We should probably be "await"ing our asynchronous methods rather than locking the thread.
• We need better error handling.
• I'm pretty sure there's a better way to get to the "sunset" value in the JSON data.
• And the parsing of the time string... I'm not sure what it needs, but it definitely needs something.

We can cross off the last item. But let's fix some of these other things -- primarily the organization of the code.

Separation of Concerns
The biggest problem I have with the method is that it is doing too many things. So I split things out a bit. In doing this, I also created a bit of a fluent API, and we'll see this as we go along.

So, here's the new "GetSunset" method:

Yep, that's it. I've split out the functionality into separate methods. This gives us smaller methods that are responsible for doing only one thing. This makes things easier to debug, maintain, and test in the long run.

New Feature: A Local Cache
You'll notice that the first thing we do is call a "RefreshCache" method. Since we are making a service call to get our sunset data, it makes sense to only do this once per day. If we need to make multiple calls, then we can just use the data that we collected earlier.

The cache is pretty simple: just 2 fields. The "cacheData" field holds the string value that comes back from the service (the JSON result in this case). The "cacheDate" is the date that the data refers to.

So, to refresh the cache, we just check to see if our "cacheDate" is the same as the date that we're looking for. If it is, then we can just use the "cacheData" that we already have. Otherwise, we make the service call to update the data.

As a side note, the commented out line is a JSON result from the service. This is so we can easily uncomment this line and comment the next line if we don't want to hit the service while we're in development mode.

Concern #1: Calling the Service
The first concern that we split into its own method is calling the service. This is the "GetContentFromService" method:

This method holds the functionality to get the JSON result from the service. For this, it creates an HttpClient, configures the query string, and then gets the results.

There are a few things we want to look at here: first, our "using" statement that wraps the "HttpClient". "HttpClient" implements the IDisposable interface, so it's a best practice to wrap this with a "using" statement. This makes sure that the Dispose method is called appropriately when we're done with the object.

A nice thing about moving this to a separate method is that the scope for the "using" is much smaller. If we look at the original method, we see that the "using" block encompasses the entire method. But since we've split this functionality out, the "using" block can be scoped more appropriately.

Another thing to notice are the "await"s inside the method. The "GetAsync" and "ReadAsStringAsync" methods are asynchronous (as you might guess from the names). Previously, we were checking the "Result" of these methods. This has the effect of locking up the thread while we wait for the calls to finish. When we "await" the methods, the thread is free to continue processing requests while we're waiting for the asynchronous methods to return. This is especially useful when we're making network calls since it usually takes some time to establish the connection and transmit the data across the wire.

And notice that since we're "await"ing methods, we have marked the "GetContentFromService" methods as "async" and it returns a "Task<string>" (rather than just a string). For more information on consuming asynchronous methods, check out Task and Await: Consuming Awaitable Methods.

Waiting for a Result
We're not taking full advantage of the asynchronous method calls. Lets go back to the "RefreshCache" method:

This just gets the "Result" from the asynchronous method. So we do end up turning this back into synchronous code. And that's what we need in this case.

Concern #2: Parsing JSON
The second big concern in our original method was to parse the JSON result that comes back from the service. We need to be able to pick out the "sunset" value from this object. And that's what we do in the "GetSunsetString" method:

This is pretty much the same code that we had before; it's just separated into its own method. This is responsible for deserializing the JSON object and then getting the value of the "sunset" field.

As a reminder, this is a string like "1:06:09 AM". This is a UTC time value that needs to be converted into a local date/time. And that's our next concern.

Concern #3: Parsing UTC Time
To parse the string time value, we have another method -- GetLocalTime:

This has the code that Matt helped us out with before. It takes the UTC string value and turns it into a DateTime object with the local sunset time. (Check the previous article for a detailed description.)

A Fluent API
I made a decision to create a fluent API with some extension methods here. Both "GetSunsetString" and "GetLocalTime" are extension methods on the string object. (For more information on extension methods, take a look at "Quick Byte: Extension Methods".)

This may not be the best direction to go. The methods themselves are very targeted. For example, "GetSunsetString" doesn't really extend any "string" object, just string objects that contain the JSON object that we're expecting.

In the same way "GetLocalTime" will only work with strings that contain a UTC time value. These aren't re-usable outside of this class.

But they do allow us to write code like this:

And I kind of like this. "cacheData" contains our JSON string, so we can call "GetSunsetString" on this object. Since the result of "GetSunsetString" is a UTC time string, we can just pass the result on to the "GetLocalTime" method.

I'm not completely sold on this. But I'll stick with it for the time being.

[Update 1/26/2015: The more I look at these extension methods, the less I like them. I'll explain why in an upcoming article... And here it is: Fixing Problems with a (Specific) Fluent API.]

Re-Using These Methods
One of the great things about having the concerns separated out into their own methods is that we can compose them in different ways. For example, I added a method that knows how to get sunrise times as well:

We have to do pretty much the same steps if we want to get a "sunrise" time instead of a "sunset" time. So we call "RefreshCache" (which calls "GetContentFromService" if needed). And we use the same "GetLocalTime" method.

The "GetSunriseString" method is a little different because it needs to get a different value out of the JSON result, but the rest of the functionality (including the cache) is shared.

Relative Schedules
To make use of the ability to get sunset and sunrise times, I modified the schedule objects a bit to support relative times. This included some other refactoring as well. Here are the objects that we have now:

Inside the "ScheduleInfo" object (I really hate that name, by the way -- that needs to be changed), there is a "ScheduleTimeType" property. This is an enum with the values "Standard", "Sunset", and "Sunrise". In addition, I added a "RelativeOffset" so that we can schedule items relative to sunset or sunrise.

Here's are some schedule items (from our "Schedule.txt" file):

The 2nd and 3rd items are scheduled relative to "Sunset". The 2nd item is 30 minutes before sunset, and the 3rd item is 1 hour after sunset.

When we run our application and check the schedule items, we can see the relative values:

At the top, we can see that the sunset time is "5:16:46 PM" (local). Notice that the 2nd item is scheduled for "4:46:46 PM" -- 30 minutes prior to sunset, and the 3rd item is scheduled for "6:16:46 PM" -- 1 hour after sunset.

So we can see that the relative schedules are working and that they are getting the sunset value from our service.

In addition, if we set a breakpoint inside the "GetContentFromService" method, we'll see that the method is only called 1 time (even though this calls "GetSunset" 3 times). This lets us know that the cache is working as expected.

How Are We Doing?
So we have working code. That's good. I put this code into production, and the items are rescheduling as expected. But let's go back and review the items that we want to fix:

We probably want to split out functionality into separate methods (service call, JSON parsing, date/time manipulation).
CLOSED: We took care of this one. We have separated out the concerns into separate methods. This will make things easier to debug and enhance. And as we saw, we can re-use the methods by composing them in different ways.

The Lat/Long values need to be a bit more dynamic (probably coming from configuration).
OPEN: I'm not overly concerned about this particular item since this software only runs at a single location, but this would be a "nice to have".

We should probably be "await"ing our asynchronous methods rather than locking the thread.
CLOSED: We added the "await"s to our HttpClient calls. We still turn this back into synchronous code, but we need to do that at some point for this code to work as expected.

We need better error handling.
PENDING: Error handling is almost non-existent, but we do check for status and null along the way. If we have a problem -- with the service call or the parsing -- then the method simply returns the value for midnight. We'll need to take a look at the application as a whole to see how we want to handle an error condition here.

I'm pretty sure there's a better way to get to the "sunset" value in the JSON data.
OPEN: I still need to do some more research on this one (or find a JSON expert to give me some advice).

And the parsing of the time string... I'm not sure what it needs, but it definitely needs something.
CLOSED: We've saw how much better the code got previously with some help from Matt.

Wrap Up
So overall, we're in much better shape than when we started. I'm still working on getting the tests in place for this functionality. And I'm also looking at different approaches to the same problem. What would happen if I had approached this entire functionality using TDD? There may be some interesting issues since we're making a service call and a big part of figuring things out was how to work with the service. I'll probably experiment with this as part of a coding practice session in the future.

Readable code is vital to keeping our technical debt manageable. Often we end up throwing code together just to get something that works. But we need to make sure that we don't leave it that way. If I had left the ugly code and come back to it 6 months later, it would have taken me a while to figure out what it is doing. But now that the different pieces of functionality are split out into their own methods, its much easier to see what's going on and how the various pieces fit together.

We need to remember that the code we write is just as much for the human developers as it is for the compiler and computer.

Happy Coding!

January & February 2015 Speaking Engagements

I'll be speaking at several events during February (and I also have a bit of a late addition for January). It should be a lot of fun.

Wednesday, January 21, 2015
Pasadena .NET Developers Group
Pasadena, CA
Meetup Link
Topic: TDD / Unit Testing / Smart Unit Tests

Tuesday, February 3, 2015
LA C# User Group
Pasadena, CA
Group Website
Topic: Abstract Art: Getting Things "Just Right"

Wednesday, February 4, 2015
So Cal .NET Developers Group
Buena Park, CA
Group Website
Topic: Learn the Lingo: Design Patterns

Saturday, February 7, 2015
South Florida Code Camp
Ft. Lauderdale, FL
Code Camp Site
Topics:
o Learn to Love Lambdas
o Learn the Lingo: Design Patterns
o Abstract Art: Getting Things "Just Right"

Wednesday, February 11, 2015 Late Addition!
vNext Orange County
Newport Beach, CA
Meetup Event
Topic: Keep Your UI Responsive with the BackgroundWorker Component

Saturday, February 21, 2015
Las Vegas Code Camp
Las Vegas, NV
Code Camp Site
Topic: Learn to Love Lambdas

I've got more events coming up in March and April in different parts of California, including San Diego, Berkeley, and Fresno. And I've also got some tentative items on my calendar, so stayed tuned as things get confirmed.

As usual, if you'd like me to come to your event, just drop me a note. You can see my current list of topics here: Jeremy's Presentation Topics.

I hope to see you at an upcoming event.

Happy Coding!

Code Gets Better by Sharing Experience and Expertise

I wrote some really ugly code yesterday. But it worked, so that's a plus. Fortunately, I've built a pretty good network of developers over the last several years. So, I knew who I could ping for some help.

A big part of the ugly code was related to date/time processing. Fortunately, I know Matt Johnson (find him on GitHub, Twitter, and Pluralsight).

I pinged him on Twitter, and he responded with with a Pull Request on the GitHub repo (which I merged in this morning). This was more than I was expecting, and it is greatly appreciated. Let's see how the code got better.

This code is currently on the "sunset" branch. Once I get the rest of the functionality in place, I'll merge this in with "master". The repo is jeremybytes/house-control on GitHub.

A Simple Change
The first change was pretty simple: just a format string when we're building the URL query string.

Original Code

New Code

Having the format string for the date field is much cleaner than having the separate fields. (I'm kind of hoping that I would have made this change myself after looking at the code a bit more.)

But the other changes are things I would not have made on my own.

Better DateTime Parsing
As a reminder, we make a call to a service that returns the "sunset" time for a particular date and location. The time is returned as UTC, so when I use my location (in Southern California), the sunset time is something like "1:06:09 AM". We need to get this into local time somehow.

Yesterday, I described this code (so I won't do it again). I'll just leave it here as a reminder:

Original Parsing

This takes the "1:06:09 AM" and turns it into "Jan 14, 2015 5:06:09 PM (Local)". It feels a bit cobbled together. And this is where Matt made things much better:

New Parsing

This condenses the 2 lines of code into a single line, and he left me a comment in GitHub to describe how this works:

When I wrote the original code, I looked at the overloads for the "DateTime.Parse" method, but I skimmed over the "DateTimeStyles" option because I wrongly assumed it wasn't what I needed. So this code has the same effect of specifying "UTC" for the incoming value and having the output still be "Local" time.

Better DateTime Creation
After getting this local time, I needed to replace the date portion with the value that was passed in to the method. The original code works, but it's pretty verbose:

Original Creation

Matt did a much better job with this:

New Creation

This is much easier to read.

A Few Assumptions
Matt also made some notes when he submitted the pull request:

The assumption is that we want to use the computer's local time zone. And this is correct for what we have right now. I have an open issue to switch to UTC (or as I should have learned from Matt by now: DateTimeOffset).

I really don't have an answer to the second part of the assumption -- whether the service is expecting local date or UTC date as an input. Quite honestly, I'm not really worried about this. Generally sunset moves a few minutes a day (with the exception of switches to/from Daylight Saving Time), so I'm not worried whether the value is for "today" or "yesterday".

The Updated Method
You can see the pull request and comments on GitHub. And it's really easy to see the differences in the file as well. GitHub is pretty cool.

Here's the updated method:

There's still a lot to do to make things more readable, but the DateTime portions are looking pretty good right now.

More About Matt
Since I've been writing about the importance of talking to other developers, I should tell you how I met Matt. Quite honestly, I'm not quite sure how long I've known him. We first met at the Desert Code Camp in the Phoenix area several years back. (The first time I went out there was 2010, so it was sometime after that.)

Matt lived in the Phoenix area at the time, and we had some good conversations about date/time and RavenDB. I've been out there 10-12 times, so we've had a lot of chances to talk over the years. I've also attended his presentations during that time. He has since moved to the Redmond area to work for Microsoft. And I got to talk to him in person a few months back when I was up there for the Microsoft MVP Summit.

Based on our conversations, I know that he's worked *a lot* with the problems that come up when dealing with date/time -- whether related to time zones, DST adjustments, or keeping up with changing international time rules. And several months ago, he released a 6 hour course on Pluralsight: Date and Time Fundamentals.

Wrap Up
It's great to have lots of friends in the developer community. Everyone has different experience with different technologies. When we build up our network, we usually end up with someone (and often several someones) we can contact for whatever problem we run into.

And the reverse is true. I have folks hit me up for help in areas that I know about. Just this week, one of my friends asked for assistance dealing with Tasks, asynchronous methods, and cancellation tokens (which was quite timely based on my current series of articles).

When we share our experience and knowledge with each other, our community grows stronger, and we all benefit with better code.

Happy Coding!

GitHub: Hand Holding Where You Need It

I'm still fairly new to Git and GitHub (been using it about a month now as you can see from my GitHub profile). And I'll have to say that I get more impressed the more I  use them.

Yesterday, I showed some really ugly code and mentioned that folks were welcome to offer suggestions through comments or by making a pull request:

Feel free to share your ideas in the comments for this article. Or make a pull request on the GitHub repo (I'm not quite sure how to deal with those since I'm still new to Git and GitHub, but I'll figure it out when it comes up).

Fortunately, my friend Matt Johnson (expert in all things date/time related) took me up on the this and made a pull request.

Help With The Pull Request
I got an email notification about the pull request, and I was happily surprised that it came with instructions:

This is what I needed: the Git command to merge the Pull Request and a bunch of links to review it.

And when I viewed it on GitHub, I got similar help. This is right in the pull request:

And when I clicked on "command line", I got step-by-step instructions:

This is exactly what's needed by someone who is still learning the tools (like me).

I took the easy route. I just clicked the "Merge pull request" button, confirmed the merge, then went to Visual Studio and hit the "Sync" button to get the changes on my local machine.

If you're curious about the changes that Matt made, don't worry. I'll be covering those soon. He made some great improvements to the code, and we'll walk through them in the next article. If you want to look at the changes in the meantime, this is all on the "sunset" branch.

[Update: Here's the review of the changes.]

Wrap Up
The more I use Git and GitHub, the more impressed I am with the features and implementation. Yesterday, I was genuinely baffled about what I would do if someone made a pull request. Today, I merged this into my project with no issues at all.

Now I'm *really* kicking myself for not doing this sooner. But I can't change the past, so I'll just keep moving forward.

Happy Coding!

First Tries Are Sometimes Ugly

Things don't always go well on the first try. That's because a big part of our job as as a developer is to do things that we've never done before. I wrote some really ugly code tonight. It works, but I'm not proud of it.

I'll be working on making this better. And if you have any ideas of how to do that, be sure to leave a comment.

This code is currently in a branch of the house-control repository on GitHub: jeremybytes/house-control Branch: sunset.

A Bright Idea
So this all started this morning. I was out for a walk listening to a podcast (MacBreak Weekly #437), and they were talking about home automation. Then someone mentioned something about having lights come on at sunset.

"That's brilliant," I thought. "Why didn't I think of that before?" You see, with my home automation software, one of the things that I end up doing is adjusting the schedule so that lights come on around the time it gets dark. I end up changing the schedule a few times a year (Daylight Saving Time start/end, plus adjustments every few months).

I immediately started thinking about a new feature for my software: be able to schedule items relative to sunrise and sunset. For example, I could have a light turn on at sunset or 2 hours after sunset. Or have lights that come on an hour before sunrise and turn off at sunrise. That didn't sound too difficult. I'd really need 2 things: a way to get sunrise/sunset information (hopefully from a public service), and a way to create relative schedule items.

Today, I worked on the first part: getting the sunset time.

A Sunrise / Sunset Service
After a quick search, I found a service that looked like it would work: http://sunrise-sunset.org/api. This is a fairly simple API. You just need to pass in the location in latitude and longitude along with a date, and it would return a JSON result.

I checked Google for the Lat./Long. of "Anaheim, CA", and started running some tests.

Note: The Lat./Long. is not for my house, so don't come looking for me there. I figured it would be for city hall, but it turns out to be about 2 miles east of city hall. So, I don't know how Google came up with this location.

Here's the output of the service (I took this screenshot from Fiddler just because it's an easy visualizer). This is based on the following call: http://api.sunrise-sunset.org/json?lat=33.8361&lng=-117.8897&date=2015-01-15

There's lots of information here. To start with, I really care about "sunset". But notice what time "sunset" is: 1:06:09 a.m.

The time is UTC. This wasn't a surprise because that's exactly what it said in the documentation. But now I needed to write some code that would run this service, parse the time out, and convert it to local time.

Calling the Service
I added a new project to the solution: "HouseControl.Sunset". In addition, I added an interface that has a single method (for now): "GetSunset(DateTime date)".

I created an interface because I figured that I would want to swap out the implementation in the future. In fact, I ran across some JavaScript libraries that calculate sunrise/sunset without a service call, so I might want to use that someday.

The implementation class is called "SunriseSunsetOrg" (since that's the site that we're hitting). I'll be the first to admit that I haven't done much work parsing JSON results, so I don't expect this is the best way to handle things.

Calling the service is not too difficult. I just set up a "HttpClient", call "GetAsync()", and parse the response. Here's the code to make the service call:

This creates the "HttpClient", configures the base address and asks for "JSON", formats the query string (like we saw in the sample URL above), and then makes the call with "GetAsync".

"GetAsync" is an asynchronous call (as you might have guessed). But instead of taking advantage of that, I'm asking for the "Result" property. This will stop execution of this thread until the asynchronous method is finished. This is similar to what we did with the console application when we first looked at consuming awaitable methods.

I could have made the "GetSunset" method asynchronous (and I may do that in the future), but for now, I just wanted to get valid data coming back.

Parsing the JSON Response
Now that we have the response, it's time to do something with it.

This uses "ReadAsStringAsync" to get the JSON content out of the response. Again, this is an asynchronous method, and we're just checking the "Result" property which will make this code wait until the asynchronous method is finished.

The "responseContent" variable now holds our JSON data. This is nicely parsed in the Fiddler screenshot above, but it really just looks like this:

As I mentioned, I don't work with JSON very much. So, I think there might be a better way to do this next part. I brought in the Newtonsoft JSON library with NuGet and deserialized into a dynamic object:

I used "dynamic" because I don't have a CLR type to use, and I really only need to get at a couple of the values, so it wouldn't make sense to create a custom type.

After getting the object, I check the "status" value. This is the very last value in the JSON. If it is not "OK", then I just return. We'll need some better error handling here.

Then I need to get to the "sunset" value. And since our "contentObject" is "dynamic", I can pretty much put in whatever I like. So, I just ask for the "results" object, and then check for the "sunset" value inside of that.

This would mean that "sunsetString" has a value of "1:06:09 AM".

Getting Local Time from a UTC Time String
This is where things get really ugly. Right now we have a string which represents a UTC time for sunset at our location. And we need to somehow turn this into a local date/time value.

Try not to cringe:

This is pretty scary. The first thing I do is create a DateTime object that will represent the UTC time. For this I call "DateTime.Parse" on the "sunsetString" value. Since the string only has a time, the date portion will be today's date. This will result in "Jan 15, 2015 1:06:09 AM".

Normally, when you do a DateTime.Parse, you end up with a local time (well, technically it is non-specified, but the default is to treat it as local time). This is no good since this represents UTC time. So I wrapped the DateTime.Parse in a "DateTime.SpecifyKind" call. This lets us specify that this is a "UTC" value. The result is that our "utcTime" value will be "Jan 15, 2015 1:06:09 AM (UTC)".

Since everything else in the system is in local time (at least for now), I need to change this to local time -- which is pretty easy with the "ToLocalTime" method. The result is that "localTime" will be "Jan 14, 2015 5:06:09 PM (Local)" -- and local time happens to be Pacific Standard Time (UTC -8).

Notice that the date part is a bit messed up since it now shows "yesterday" instead of "today". But this won't matter because we'll be discarding this part of the value.

To create the "resultDateTime" (which is the local sunset time that we want to return), we create a new DateTime object and pass in the date portions from our original parameter (the requested date), and the time portions from our sunset time.

The result is that we end up with "Jan 15, 2015 5:06:09 PM (Local)". And the good news is that this is the value that we're looking for.

Ugly Code
To really appreciate how ugly this code is, let's look at the entire method:

Things aren't always pretty the first time around. This code is functional, but it still needs a lot of work.

• We probably want to split out functionality into separate methods (service call, JSON parsing, date/time manipulation).
• The Lat/Long values need to be a bit more dynamic (probably coming from configuration).
• We should probably be "await"ing our asynchronous methods rather than locking the thread.
• We need better error handling.
• I'm pretty sure there's a better way to get to the "sunset" value in the JSON data.
• And the parsing of the time string... I'm not sure what it needs, but it definitely needs something.

As a side note, the commented "responseContent" string is the raw JSON response for this particular request. This is so we don't have to keep making the service calls while we're testing out this method (just comment out the service call and uncomment the hard-coded string).

Feel free to share your ideas in the comments for this article. Or make a pull request on the GitHub repo (I'm not quite sure how to deal with those since I'm still new to Git and GitHub, but I'll figure it out when it comes up).

[Update 1/16/2015: I know how to deal with a pull request now.]

Wrap Up
I'm not done with this code. This is just a start. I've got this on the "sunset" branch, and I won't merge this back to "master" until it's in a better state. This includes doing some clean up that I know how to do and also tapping into some friends for things I don't know how to do.

The moral of the story is that we don't always get pretty code the first time we try something. And that's okay. It's all part of the learning process. As developers, we're constantly doing things that we've never done before. It's a bit scary, at times we get get things wrong, but it's a pretty cool job to have.

Happy Coding!