Running JavaScript... With Sneakers!

Code-review time. I haven't written significant JavaScript in forevs, but I hit upon a use case well suited to it, had a blast coding it up, and am confident that I'll be completely mystified by it three months from now. That is, unless we refactor it for maintainability (this is where you come in).

I'll explain the use case. The Couch-to-5K running plan is an excellent exercise regimen for learning to run. I followed it to successfully transform myself from sedentary software developer to 5K finisher. The runners among you know that five kilometers is a beginner-level distance; the sedentary software developers may find it as daunting as I once did. You train in intervals, alternating walking and running, gradually shifting the ratios to more running in successive workouts. If you can walk a moderate distance, following the program is totally doable (or modify it for easy walking/fast walking, if you're not up for jogging). Definitely check it out.

The thing is, timing the intervals—especially at the beginning, where you're alternating a minute of running with 90 seconds of walking—needs a complex timing device (a fancy runners watch), and if you're using a treadmill, it is really annoying to your fellow gym users, as your watch is beeping every minute or two. Or, well, maybe gym people don't care, but it's really annoying to me, since the dang thing is on my own wrist. For running indoors, I want a C25K timer that is tactile or visual. I've been thinking about an Arduino-powered timer that reads from a dip switch to determine which workout you're running and signals the changes with a vibration motor (like in a cell phone). Thinking, and not so much doing. Finally one Sunday I forced myself to focus on the problem: what do you have, with a visual display, that is really small and runs code?

My Nokia N810 internet tablet has a web browser that runs JavaScript, can open html files stored locally, and can do this while also playing a podcast. Win!

Here's what you can't do with JavaScript, though: Thread.Sleep(walkInterval). Instead, you use setTimeout() to say "execute this function after this interval," asynchronously. That's the part that currently works but I daren't touch it. In other words, that's the part that needs your code review advice. Remember that the intended scenario is to rest the tablet on the treadmill's magazine lip, so the app flashes the background of the page to catch my eye when I'm not looking directly at it. It's high-contrast because it isn't meant to be watched, just kept at hand, in my periphery.

If you choose to use the app (in addition to reviewing it), please only use it on a treadmill. Stay safe; don't try to wrangle something you need to look at while you're on the trail.

The Running Timer code is on github. A representative sample is below. How would you make it better?

var exercise = function(setup) {
 var workout = getWorkout(setup.week, setup.day);
 doExercise(workout, 0);
}

var cooldown = function() {
 walk('Cool Down');
 window.setTimeout(function() { transition(function() { walk('Done!'); })}, 
  minToMilli(RT.cooldownDuration));
}

var doExercise = function(workout, i) {
 workout[i].mode();
 if (i === workout.length - 1) {
  window.setTimeout(function() { transition(cooldown) }, 
   minToMilli(workout[i].minutes));
 } else {
  window.setTimeout(function() { transition(function() { doExercise(workout, i + 1); })}, 
   minToMilli(workout[i].minutes));
 }
}

var getWorkout = function(week, day) {
 return eval('C25K.W' + week + 'D' + day);
}

var transition = function(callback) {
 var indicatorDiv = $('.runCountdown');
 var body = $(document.body);
 transitionBlink(6, indicatorDiv, body, callback);
}

An Object Lesson in Binary Compatibility

A riddle for you, friends: When is changing a method from return void to return Something a breaking change?

If you already know the answer, then why hadn't you told me? Could've saved me a fair bit of embarrassment. Ah well, maybe I missed your call.

First, a story

I had the opportunity to contribute to the HtmlTags open-source project1. They needed some unit test coverage, and I <3 unit testing and wanted to try my hand at contributing to open-source software. As I got my bearings, I got more confident and started to reach beyond the unit test project into refactoring the application code.

That's when I found the AddStyle and AddJavaScript methods. Their friends returned an HtmlTag, but they returned void. They would be easier to test if they returned an HtmlTag, but that's not a good justification for changing a method. But they would be more consistent if they behaved like their neighbors, and that is a sufficient justification to consider it.

HtmlTags is a library meant to be consumed by other applications. It was already in use in the wild. Therefore, it was important not to change its API. It had to work like it always had, else we'd cause considerable grief to the developers using our code. I've been the consumer of a service that made breaking changes to its API, and I have choice things to say about the team that saddled me with that noise.

I thought really carefully. I was contemplating changing the return type of a public method in an in-use API. But all those uses would be calling it as if it returned nothing, and you can invoke a method without using its return value. This would be just like that. I asked a coder-buddy for advice. We convinced each other: What harm could it do?

What about the riddle?

Here we come to the answer to the riddle. If you're changing a library that will be called by other applications, then there are many seemingly harmless changes that are actually breaking changes, including changing a method's return type. When the consuming assembly is compiled, it is built with instructions to find a method named Whatever that returns void, but the changed assembly's manifest contains only a method named Whatever that returns string. No match.

Computers. They're so literal.

To fix it, you merely need to recompile the consuming application, but you don't discover the problem until assemblies are loaded at run-time, an annoying time to discover exceptions. I coded up a simple example that illustrates the problem. Feel free to follow along at home.

See the problem in action

Make two separate solutions, a class library called MessageOutputter and a console application called ConsoleMessager that references MessageOutputter. First, build MessageOutputter with a return void method.

namespace MessageOutputter
{
  public class Outputter
  {
    private string _message = "This is the outputter, version 1.";

    public string Emit()
    {
      return _message;
    }

    public void UpdateVersion(string versionNumber)
    {
      _message = string.Format("I've been altered by the UpdateVersion method. I am version {0}.", versionNumber);
    }
  }
}

UpdateVersion modifies a private field but does not return a value. Compile that solution and put its dll into a lib folder from which you will reference it in ConsoleMessager. Open the ConsoleMessager solution and add a reference to the MessageOutputter.dll. Write a method that uses the MessageOutputter.

namespace ConsoleMessager
{
  class Program
  {
    static void Main(string[] args)
    {
      var outputter = new MessageOutputter.Outputter();
      Console.WriteLine("Calling the outputter:");
      Console.WriteLine(outputter.Emit());
      Console.WriteLine("Asking the outputter to update, then calling it again.");
      outputter.UpdateVersion("Updated 1");
      Console.WriteLine(outputter.Emit());
      Console.Read();
    }
  }
}

The UpdateVersion method is called, not expecting a return value. Build and run the ConsoleMessager.



Return to the MessageOutputter solution and modify the UpdateVersion method to return the message after it modifies it.

namespace MessageOutputter
{
  public class Outputter
  {
    private string _message = "This is the outputter, version 2.";

    public string Emit()
    {
      return _message;
    }

    public string UpdateVersion(string versionNumber)
    {
      _message = string.Format("I've been altered by the UpdateVersion method. I am version {0}.", versionNumber);
      return _message;
    }
  }
}

Now UpdateVersion returns a string. Build the solution and copy its new dll back into the lib folder, and into the Debug or Release folder under ConsoleMessager/bin, so that the ConsoleMessager will run with your new version of the dll. Run ConsoleMessager and you will encounter the error:

Unhandled Exception: System.MissingMethodException: Method not found: 'Void MessageOutputter.Outputter.UpdateVersion(System.String)' at ConsoleMessager.Program.Main(String[] args)

The MissingMethodException indicates that ConsoleMessager, when looking within the MessageOutputter library, could not find an UpdateVersion method that returns void. Reopen the ConsoleMessager solution. Before you even build, IntelliSense will tell you that UpdateVersion returns a string now. Build ConsoleMessager and run it again, to see that it works successfully.

Wiser now

No one raked me over the coals—in fact, my intro-to-OSS experience was thoroughly positive, and I can't wait to do more—but I know I caused some time-consuming inconvenience to my fellow devs. I have now learned that Binary Compatibility is not the same as Source Compatibility. Conflating the two is like saying, "Works on my machine." I hope this write-up can help you avoid a similar goof.

1 The nice thing about open-source projects is that everyone has the opportunity to contribute. But I had the especially good fortune to have willing help from Josh Flanagan in finding my way through my first OSS pull request.

Refactoring Dinner: Interfaces instead of Inheritance

Last time, in Cooking Up a Good Template Method, I had a template method cooking our dinner. An abstract base class defined the template—the high level steps for preparing a one-skillet dinner—and a derived class provided the implementation for those steps. I'm currently reading Ken Pugh's Interface Oriented Design (more on that after I finish the book), and it got me thinking of a way to change the design to use interfaces instead of inheritance.

I think there's value in this refactoring because it allows future flexibility and testability. Let's stroll through it, and I welcome your thoughts about how (and whether) this improves the code.

Previously, we had a base class SkilletDinner, which was extended by variants on that theme, such as chicken with onions and bell peppers or the FancyBaconPankoDinner. (If I've learned one thing from my readership, it is that blog posts should mention bacon. Mm, crispy bacon.) As the first step in the refactoring, I'll create an interface, ISkilletCookable that provides the same methods that were previously abstract methods in SkilletDinner. By naming convention, the interface is prefixed with 'I' and is an adjective describing how it can be used (-able).

    4   public interface ISkilletCookable

    5   {

    6     void HeatFat();

    7     void SauteSavoryRoot();

    8     void SauteProtein();

    9     void SauteVegetables();

   10     void AddSauceAndGarnish();

   11   }

Next, I'll create a SkilletDinner constructor that accepts an ISkilletCookable, and change the SkilletDinner's Cook() method to ask that cookable to do the work. SkilletDinner no longer needs to be abstract.

    5   public class SkilletDinner

    6   {

    7     private readonly ISkilletCookable cookable;

    8 

    9     public SkilletDinner(ISkilletCookable cookable)

   10     {

   11       this.cookable = cookable;

   12     }

   13 

   14     public void Cook()

   15     {

   16       cookable.HeatFat();

   17       cookable.SauteSavoryRoot();

   18       cookable.SauteProtein();

   19       cookable.SauteVegetables();

   20       cookable.AddSauceAndGarnish();

   21     }

   22   }

Then, FancyBaconPankoDinner implements ISkilletCookable and provides implementations for each of the methods that will be called by the Cook() method.

The first benefit from this refactoring is flexibility. While FancyBaconPankoDinner could not have inherited multiple classes (no multiple inheritance in C#), it can implement multiple interfaces. For example, it could also implement the IShoppable interface, thereby providing a ListIngredients() method that would let me include it in my grocery list.

This refactoring also makes it easier for me to test the quality and completeness of my template method. I can verify—does it cover all of the requisite steps for cooking a skillet dinner?—by creating behavior-verifying tests that assess the SkilletDinner's interactions with the ISkilletCookable interface. When I'm writing unit tests for the SkilletDinner class, I want to test its behavior because the behavior is what's important.

To forestall objections, I tried writing a test around the old version, creating my own mock class that extends the old abstract SkilletDinner. It got pretty lengthy.

    4   public class SkilletDinnerSpecs

    5   {

    6     [TestFixture]

    7     public class When_told_to_cook

    8     {

    9       const string heatFatMethod = "HeatFat";

   10       const string sauteSavoryRootMethod = "SauteSavoryRoot";

   11       const string sauteProteinMethod = "SauteProtein";

   12       const string sauteVegetablesMethod = "SauteVegetables";

   13       const string addFinishingTouchesMethod = "AddFinishingTouches";

   14 

   15       [Test]

   16       public void Should_follow_dinner_preparation_steps_in_order()

   17       {

   18         var systemUnderTest = new MockSkilletDinner();

   19 

   20         var expectedMethodCalls = new List<string>();

   21         expectedMethodCalls.Add(heatFatMethod);

   22         expectedMethodCalls.Add(sauteSavoryRootMethod);

   23         expectedMethodCalls.Add(sauteProteinMethod);

   24         expectedMethodCalls.Add(sauteVegetablesMethod);

   25         expectedMethodCalls.Add(addFinishingTouchesMethod);

   26 

   27         systemUnderTest.Cook();

   28 

   29         Assert.AreEqual(expectedMethodCalls.Count, systemUnderTest.CalledMethods.Count, "Expected number of called methods did not equal actual.");

   30 

   31         for (int i = 0; i < expectedMethodCalls.Count; i++)

   32         {

   33           Assert.AreEqual(expectedMethodCalls[i], systemUnderTest.CalledMethods[i]);

   34         }

   35       }

   36 

   37       private class MockSkilletDinner : SkilletDinner

   38       {

   39         public readonly List<string> CalledMethods = new List<string>();

   40 

   41         protected override void HeatFat()

   42         {

   43           CalledMethods.Add(heatFatMethod);

   44         }

   45 

   46         protected override void SauteSavoryRoot()

   47         {

   48           CalledMethods.Add(sauteSavoryRootMethod);

   49         }

   50 

   51         protected override void SauteProtein()

   52         {

   53           CalledMethods.Add(sauteProteinMethod);

   54         }

   55 

   56         protected override void SauteVegetables()

   57         {

   58           CalledMethods.Add(sauteVegetablesMethod);

   59         }

   60 

   61         protected override void AddFinishingTouches()

   62         {

   63           CalledMethods.Add(addFinishingTouchesMethod);

   64         }

   65       }

   66     }

   67   }

In the new design, I can mock the ISkilletCookable interface with a mocking framework like Rhino.Mocks. The interface is easy to mock because interfaces, being the epitome of abstractions, readily lend themselves to being replaced by faked implementations. Rhino.Mocks takes care of recording and verifying which methods were called.

    7   public class SkilletDinnerSpecs

    8   {

    9     [TestFixture]

   10     public class When_told_to_cook

   11     {

   12       [Test]

   13       public void Should_follow_dinner_preparation_steps_in_order()

   14       {

   15         var mocks = new MockRepository();

   16         var cookable = mocks.StrictMock<ISkilletCookable>();

   17         var systemUnderTest = new SkilletDinner(cookable);

   18 

   19         using (mocks.Record())

   20         {

   21           using (mocks.Ordered())

   22           {

   23             cookable.HeatFat();

   24             cookable.SauteSavoryRoot();

   25             cookable.SauteProtein();

   26             cookable.SauteVegetables();

   27             cookable.AddSauceAndGarnish();

   28           }

   29         }

   30         using (mocks.Playback())

   31         {

   32           systemUnderTest.Cook();

   33         }

   34       }

   35     }

   36   }

The test relies on Rhino.Mocks to create a mock implementation of ISkilletCookable, and then verifies that the system under test, the SkilletDinner, interacts correctly with ISkilletCookable by telling it what steps to do in what order.

That test is quite cognizant of the inner workings of the SkilletDinner.Cook() method, but that's specifically what I'm unit testing: Does the template method do the right steps? I don't mind how the steps are done, but you have to start the onions before you add the meat, or else the onions won't caramelize and flavor the oil.

By the way, if you had previously found the learning curve for Rhino.Mocks' record/playback model too steep a hill to climb (or to convince your teammates to climb), check out Rhino.Mocks 3.5's arrange-act-assert style. It creates more readable tests, putting statements in a more intuitive order. I really like it. I could not, however, use it here because I have not found a way to enforce ordering of the expectations (i.e., to assert that method A was called before B, and to fail if B was called before A) in A-A-A-style. So we have a record/playback test, instead.

Here's a summary of the refactoring. I extracted an interface, ISkilletCookable, and composed SkilletDinner with an instance of that interface, liberating us from class inheritance. Because SkilletDinner is now given the worker it depends on (via dependency injection), I can give it a fake worker in my tests, so that my unit tests don't need to perform the time- and resource-consuming operation of firing up the stove. And I managed to write another blog post that mentions bacon. Mm, bacon.