My journey to BDD using NBehave

In the beginning, there was some code:

   1: public class Calculator
   2: {
   3:     private int _accumulator;
   4:  
   5:     public int Result
   6:     {
   7:         get { return _accumulator; }
   8:     }
   9:  
  10:     public void Add(int n)
  11:     {
  12:         _accumulator += n;
  13:     }
  14:  
  15:     public void Subtract(int n)
  16:     {
  17:         _accumulator -= n;
  18:     }
  19: }

[Well obviously the code I had written was a bit more complicated than this … but this should do for the sake of illustration]

And I realised that code was legacy code, and so I wrote some unit tests:

   1: [TestClass]
   2: public class CalculatorTests
   3: {
   4:     [TestMethod]
   5:     public void TestTheCalculator()
   6:     {
   7:         var calc = new Calculator();
   8:         calc.Add(10);
   9:         Assert.AreEqual(10, calc.Result);
  10:         calc.Add(5);
  11:         Assert.AreEqual(15, calc.Result);
  12:         calc.Subtract(7);
  13:         Assert.AreEqual(8, calc.Result);
  14:     }
  15: }

And after writing quite a few tests in this style, I started to realise just how horrible they are.  Firstly, the name of the test tells me nothing at all; secondly, the various operations and assertions all depend on each other; and thirdly, I need to read and understand all the code to actually get my head around the intent of the test.  And so I started using a new, simpler style:

   1: [TestClass]
   2: public class CalculatorTests
   3: {
   4:     [TestMethod]
   5:     public void Add_AddsNumberToTheAccumulator_ResultIsCorrect
   6:     {
   7:         var calc = new Calculator();
   8:         calc.Add(10);
   9:         Assert.AreEqual(10, calc.Result);
  10:     }
  11:  
  12:     [TestMethod]
  13:     public void Subtract_SubtractsNumberFromTheAccumulator_ResultIsCorrect
  14:     {
  15:         var calc = new Calculator();
  16:         calc.Subtract(7);
  17:         Assert.AreEqual(-7, calc.Result);
  18:     }
  19: }

Better!  But what if the Add method has some nasty side effect that results in it breaking on the third call?  And what if there is some quirk in the Subtract method that means it will not operate on the number 13? (IOW what happens when the tests get more complex).  In the same style as above, the tests become:

   1: public class Calculator
   2: {
   3:     public Calculator() { }
   4:  
   5:     public Calculator(int start) 
   6:     {
   7:         _accumulator = start;
   8:     }
   9:  
  10:     // Other code omitted for clarity
  11: }
  12:  
  13: [TestClass]
  14: public void CalculatorTests
  15: {
  16:     [TestMethod]
  17:     public void Add_RepeatedAddition_CheckThatThreeTriesDoNotFail()
  18:     {
  19:         var calc = new Calculator();
  20:         calc.Add(10);
  21:         calc.Add(10);
  22:         calc.Add(10);
  23:         Assert.AreEqual(30, calc.Result);
  24:     }
  25:  
  26:     [TestMethod]
  27:     public void Subtract_ApplyToThirteen_ResultIsStillCorrect()
  28:     {
  29:         var calc = new Calculator(13);
  30:         calc.Subtract(5);
  31:         Assert.AreEqual(8, calc.Result);
  32:     }
  33: }

The test relating to addition looks a little … odd.  If I hadn’t written that test then I honestly wouldn’t know what the point of it was.  And the second test?  Well, you notice that we are passing in some state and assigning it to our private field.  It’s very common to do that with Dependency Injection, but this is state, not a dependency, and so we should question it closer.  It becomes an increasingly valid concern as the state gets more and more complex.  After some careful thought, I removed the new constructor and rewrote these tests as:

   1: public void Calculator
   2: {
   3:     public void Reset()
   4:     {
   5:         _accumulator = 0;
   6:     }
   7:  
   8:     // Other code omitted for clarity
   9: }
  10:  
  11: [TestClass]
  12: public void CalculatorTests
  13: {
  14:     [TestMethod]
  15:     public void Add_RepeatedAddition_CheckThatThreeTriesDoNotFail()
  16:     {
  17:         // ARRANGE
  18:         var calc = new Calculator();
  19:         calc.Add(10);
  20:         calc.Add(10);
  21:  
  22:         // ACT
  23:         calc.Add(10);
  24:  
  25:         // ASSERT
  26:         Assert.AreEqual(30, calc.Result);
  27:     }
  28:  
  29:     [TestMethod]
  30:     public void Subtract_ApplyToThirteen_ResultIsStillCorrect()
  31:     {
  32:         // ARRANGE
  33:         var calc = new Calculator();
  34:         calc.SetupToBeThirteen();
  35:  
  36:         // ACT
  37:         calc.Subtract(5);
  38:  
  39:         // ASSERT
  40:         Assert.AreEqual(8, calc.Result);
  41:     }
  42: }
  43:  
  44: public static CalculatorExtensions
  45: {
  46:     public static void SetupToBeThirteen(this Calculator calc)
  47:     {
  48:         calc.Reset();
  49:         calc.Add(13);
  50:     }
  51: }

That certainly feels better.  I no longer have a class that is willy-nilly accepting state from unknown others, I’ve added a method which feels like it should be there anyway, my setup code is neatly wrapped up into a reusable and readable extension method, and the intent of the addition test is much clearer.

Now I’m of the opinion that once you grok and apply the AAA (Arrange, Act, Assert) syntax, then BDD is largely just a translation of terms you already know.  But if it’s so simple, then why bother at all?  Well, I found that using NBehave I can rewrite the above tests like so:

   1: [TestClass]
   2: public void CalculatorTests : ScenarioDrivenSpecBase
   3: {
   4:     protected override Feature CreateFeature()
   5:     {
   6:         return new Feature("arithmetic calculation")
   7:             .AddStory()
   8:             .AsA("user")
   9:             .IWant("my calculator to perform basic arithmetic")
  10:             .SoThat("I don't need to try and do it in my head");
  11:     }
  12:  
  13:  
  14:     [TestMethod]
  15:     public void Add_RepeatedAddition_CheckThatThreeTriesDoNotFail()
  16:     {
  17:         Calculator calc = null;
  18:  
  19:         Feature.AddScenario()
  20:             .Given("a calculator",        () => calc = new Calculator())
  21:             .And("I have added 10",       () => calc.Add(10))
  22:             .And("I have added 10 again", () => calc.Add(10))
  23:             .When("I add another 10",     () => calc.Add(10))
  24:             .Then("the sum should be 30", () => Assert.AreEqual(30, calc.Result));
  25:     }
  26:  
  27:     [TestMethod]
  28:     public void Subtract_ApplyToThirteen_ResultIsStillCorrect()
  29:     {
  30:         Calculator calc = null;
  31:  
  32:         Feature.AddScenario()
  33:             .Given("a calculator",                () => calc = new Calculator())
  34:             .And("which has a total of 13",       () => calc.SetupToBeThirteen())
  35:             .When("I perform a subtraction",      () => calc.Subtract(5))
  36:             .Then("the result should be correct", () => Assert.AreEqual(8, calc.Result));
  37:     }
  38: }
  39:  

And to me this represents a big step forward, even though it’s pretty easy to recast the previous tests in this style.  We are now linking tests directly to scenarios, and scenarios directly to user stories.  It becomes immensely clear why this test is present and what it’s hoping to achieve.  And on top of that, the tests become immensely readable.

[I would actually rename these tests before I was happy … I prefer the “ShouldDoSomething” style of test name … and there are also much cleaner ways of coding the assertions]

You may ask if this level of attention to detail is appropriate in tests around a simple calculator class.  The craftsman in me insists that if it’s worth doing, then it’s worth doing properly.  But that aside, my current project (4,000 tests and counting) has found that if tests are not scrupulously well written then they become a source of friction.  Applying this style of BDD reduced the friction and took away the drag factor.

Interestingly there are actually other ways of casting these tests with NBehave, and I will be posting about those soon, but this concludes the story of my personal journey into BDD.

PS. This NBehave syntax is available in the trunk and will be included in version 0.5.  If you can’t wait then you can download the assemblies from the build server.

April 7 2010

Memory Leaks and Dependency Properties

As we all know (or perhaps not) WPF introduced something called the “Dependency Property”.  This is a way of externalizing state from controls.  So for example, the text value of a TextBox is actually not stored within a private field – instead the TextBox has requested the WPF framework to store the value.

This approach is clearly reducing the encapsulation of the controls, however there are some very notable benefits from doing this:

  1. WPF is aware when values change, and so can update UI nicely (i.e. databinding)
  2. WPF can update values safely (e.g. due to animation) and can choose to pause updates onto an object
  3. WPF can optimise storage of these values (e.g. by only storing values which differ to default)
  4. Values can cascade along the visual control tree, thus allowing styling, inherited values and so on

Overall they are a big win for WPF.  Here is a typical pattern for implementing a dependency property:

   1: public static readonly DependencyProperty FooProperty = 
   2:     DependencyProperty.Register("Foo", typeof(string), typeof(MyUserControl));
   3:  
   4: public string Foo
   5: { 
   6:     get { return (string)GetValue(FooProperty); } 
   7:     set { SetValue(FooProperty, value); } 
   8: } 


So how can we intercept property updates?

Obviously we can put code into the setter of the property … but given point 2 above (i.e. WPF will sometimes update the values directly without calling our object code) this will not cover all eventualities. And so the WPF team came up with a way of notifying our code when a value changes – the DependencyPropertyDescriptor.  You’ll often see samples around the web like this:

   1: DependencyPropertyDescriptor dpd = 
   2:     DependencyPropertyDescriptor.FromProperty(MyUserControl.FooProperty, 
   3:                                               typeof(MyUserControl)));
   4: if (dpd != null)
   5: {
   6:     dpd.AddValueChanged(ControlInstance, delegate
   7:     {
   8:         // Add property change logic.
   9:     });
  10: }


and this will now leak memory.

And where is the memory leak?

Whenever you attach an object onto an event of another object you introduce the potential for a memory leak.  This WPF is fundamentally no different.  The use of the AddValueChanged method has introduced a memory leak, since the object containing our code is now referenced by the dependency property and so will never get garbage collected.

So we need to remove the event handler when …

Exactly … when should we remove the event handler?  Event handlers are typically removed as part of the Dispose pattern, and then everyone tries really hard to make sure views and other objects get disposed properly.  WPF gives us another, much cleaner and more reliable option:

   1: private static readonly DependencyProperty FooPropertyDescriptor = 
   2:     DependencyProperty.RegisterAttached("FooDescriptor", 
   3:                                         typeof (DependencyPropertyDescriptor), 
   4:                                         typeof (MyUserControl));
   5:  
   6: private void AttachEventHandlers() 
   7: { 
   8:     DependencyPropertyDescriptor dpd = 
   9:        DependencyPropertyDescriptor.FromProperty(GridViewColumn.WidthProperty, 
  10:                                                  typeof (GridViewColumn)); 
  11:     if(dpd != null) 
  12:     { 
  13:         dpd.AddValueChanged(source, OnFooChanged); 
  14:         source.SetValue(FooPropertyDescriptor, dpd); 
  15:     } 
  16: }
  17:  
  18: private void OnFooChanged(object sender, EventArgs arg) 
  19: { 
  20:     var source = (DependencyObject)sender; 
  21:     if (!BindingOperations.IsDataBound(source, FooProperty)) 
  22:     { 
  23:         // We are no longer data-bound, so we need to remove the ValueChanged 
  24:         //    event listener to avoid a memory leak 
  25:         var dpd = (DependencyPropertyDescriptor)source.GetValue(FooPropertyDescriptor); 
  26:         if(dpd != null) 
  27:         { 
  28:             dpd.RemoveValueChanged(source, OnFooChanged); 
  29:             source.SetValue(FooPropertyDescriptor, null); 
  30:         } 
  31:         return; 
  32:     }
  33:  
  34:     // Add property change logic 
  35: }
  36:  


Explanation:

  1. We register a private DependencyProperty of type DependencyPropertyDescriptor
  2. When we register the value changed callback, we set the value of this private dependency property
  3. Whenever the value changed callback is called, then we check BindingOperations.IsDataBound.  If this returns false then we use the cached DependencyPropertyDescriptor to deregister the value changed callback.

Result:

  1. No more memory leaks

Credits and references:

A big part of this technique comes from:

http://wpfglue.wordpress.com/2009/12/03/forwarding-the-result-of-wpf-validation-in-mvvm/

For a more detailed primer on dependency properties check out:

http://compilewith.net/2007/08/wpf-dependency-properties.html

March 19 2010

Another reason to love Resharper: it can make you look like a LINQ-ninja

For those of who spend our time immersed in code, we're usually trying to find ways to write better code that more clearly expresses our intent.  And in a place like EMC Consulting there is a very healthy competition between devs to write code that is better, cleaner, faster, etc.  One of the new features of Resharper 5.0 (R#) manages to address both of these points!  [:D]

Jetbrains have given R# a pretty deep understanding of LINQ.  To be honest, I'm feeling like R# knows LINQ considerably better than I do!  You see, R# 5.0 will offer to convert your code into LINQ statements wherever it can.  Here's an example:

My code:

   1: foreach(var summaryType in TypeMapper.GetMapper(entityType.Assembly)
   2:           .GetSummaryTypes(entityType))
   3: {
   4:   if(typeof(VersionableSummaryBase).IsAssignableFrom(summaryType))
   5:   {
   6:     var summary = (VersionableSummaryBase)EntityCache.Instance.RetrieveIfCached(id, summaryType);
   7:     if (((IVersionable)entity).Version < summary.Version)
   8:     {
   9:        mustReload = true;
  10:        break;
  11:     }
  12:   }
  13: }

Applying the R# suggestion:

   1: mustReload = TypeMapper.GetMapper(entityType.Assembly)
   2:                  .GetSummaryTypes(entityType)
   3:                  .Where(summaryType => typeof(VersionableSummaryBase).IsAssignableFrom(summaryType))
   4:                  .Select(summaryType => (VersionableSummaryBase) EntityCache.Instance.RetrieveIfCached(id, summaryType))
   5:                  .Where(summary => summary != null)
   6:                  .Select(summary => summary.Version)
   7:                  .Any(summaryVersion => ((IVersionable) entity).Version < summaryVersion);

 

Even without understanding what my TypeMapper class is doing, or any of the other details you can clearly see, I think that the second version is easier to read and more clearly expresses the underlying algorithm.  If you have an opinion on this, then please leave it in the comments ... I'd love to get other views.

Here's another gem from R#.  My code:

   1: var sourceElements = new Dictionary<int, EntityBase>();
   2: foreach (EntityBase element in collectionFromSource)
   3: {
   4:     sourceElements.Add(element.InternalID, element);
   5: }

And the R# version:

   1: var sourceElements = collectionFromSource
   2:                         .Cast<EntityBase>()
   3:                         .ToDictionary(element => element.InternalID);

I didn't even know that that particular extension methods existed until R# suggested I use it!  And perhaps with R# 5.0 in my toolkit, I'll be able to get the edge over some of the other devs at work.  At least, until someone blogs about it ... oh bother!

December 2 2009

Sharding into the cloud

NHibernate.Shards is an extension to the well-known ORM which allows a logical database to be partitioned across multiple physical databases and servers.  It's a port of the Hibernate.Shards project, as with lots of thing in NHibernate.  I thought it would be interesting to see how well it worked against SQL Azure.  It turned out to be not interesting at all ... just plain easy!

Step 1: Register with SQL Azure
Turnaround on token requests is pretty quick right now (<24 hours in my case).

Step 2: Setup some SQL Azure databases

Step 3: Setup appropriate logins, users
The SQL Azure team have done a great job to allow SQL Server Management Studio to connect a query window to an Azure database, but I'm a bit SQL-phobic at the best of times. This was the most challenging bit for me!

Step 4: Download and compile NHibernate.Shards from NHContrib Subversion

Step 5: Set your connection strings in the config file

Step 6: Press play.  Really, that's all there is to it!

Now you may notice that I neglected to create any schema in the Azure databases - that's because NHibernate can do that for me.  Did I mention that I'm a bit SQL-phobic?  [;)]

The code I was using was the standard example that comes with NHibernate.Shards, which records WeatherReport objects, which I've attached.  It's the same example that Ayende dissected, so you can also pick up his discussion of hards-progress-report.aspx" mce_href="http://ayende.com/Blog/archive/2009/10/18/nhibernate-shards-progress-report.aspx">the workings of NHibernate.Shards.  The code looks like this (click to enlarge):

sql-azure-shard-code

And the results are as follows (click to enlarge):

sql-azure-shards

Some of the features of NHibernate.Shards that really stood out for me:

  • It can query all shards in parallel or sequentially.  For SQL Azure, that's quite useful!  A sequential query my single-record shards took 601ms, whereas a parallelized query took 411ms (almost 33% less).
  • New records can be allocated to shards based on either the data (e.g. surname starts with A-M or N-Z) or some other scheme (e.g. round-robin).
  • If the correct shard can be identified based on an object's identity, then only that single shard is queried to retrieve the entity (this is based on your own IShardResolutionStrategy implementation).
  • If you sort by a property, then this sort will be applied even when data is merged from multiple shards.

Overall though, it all just works tremendously well.  Congratulations really must go to:

  • The Microsoft SQL Azure team
  • Dario Quintana, for his work on NHibernate.Shards
  • Fabio, Ayende and the rest of the NHibernate committers

EDIT: Querying data from the shards is done using code like the following.  You should notice that this code makes no references to the shards, and in fact is "normal" NHibernate code.  The sharding is all handled transparently in the ORM.

sql-azure-shard-code2

November 10 2009
Newer Posts Older Posts