Monday, April 20, 2009

Eric Lippert's Comma Quibbling Challenge -- Using TDD

So Eric Libbert recently had an interesting "code challenge" blog

Here are the requirements...
Accept a non null IEnumerable

(1) If the sequence is empty then the resulting string is "{}".
(2) If the sequence is a single item "ABC" then the resulting string is "{ABC}".
(3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}".
(4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!)

I figured I'd give it a pass....

Being the TDD dev that I am I first wrote this unit test...

    6 namespace Ellemy.Utilities.UnitTests

    7 {

    8     /*

    9 (1) If the sequence is empty then the resulting string is "{}".

   10 (2) If the sequence is a single item "ABC" then the resulting string is "{ABC}".

   11 (3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}".

   12 (4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!)

   13 */

   14     [TestFixture]

   15     public class IEnumberableExtensionsTestFixture

   16     {

   17         private IEnumerable<String> testEnumerable;

   18         private string expectedResults;

   19 

   20         [TearDown]

   21         public void AssertAll()

   22         {

   23             var actualResults = testEnumerable.ToNonOxfordCommaString();

   24             Assert.That(actualResults, Is.EqualTo(expectedResults));

   25         }

   26         [Test]

   27         public void an_empty_string_returns_empty_curly_braces()

   28         {

   29             testEnumerable = new List<String> { "" };

   30             expectedResults = "{}";

   31 

   32         }


So, to make this test pass, I just need to simple write an extension method for IEnumerable that returns "{}"... That's it...


So I wrote this


    1 using System;

    2 using System.Collections.Generic;

    3 using System.Linq;

    4 

    5 namespace Ellemy.Utilities

    6 {

    7     /*

    8 (1) If the sequence is empty then the resulting string is "{}".

    9 (2) If the sequence is a single item "ABC" then the resulting string is "{ABC}".

   10 (3) If the sequence is the two item sequence "ABC", "DEF" then the resulting string is "{ABC and DEF}".

   11 (4) If the sequence has more than two items, say, "ABC", "DEF", "G", "H" then the resulting string is "{ABC, DEF, G and H}". (Note: no Oxford comma!)

   12 */

   13     public static class IEnumberableExtensions

   14     {

   15         ///

   16         /// Retuns a joined string with "and" as the last join.

   17         ///

   18         ///

   19         public static string ToNonOxfordCommaString(this IEnumerable<String> enumberable)

   20         {

   21             return "{}";

   22         }


But Elliott!! That doesn't work for anything but the empty string! 


Of course it doesn't! Because I don't have a unit test for that... So I wrote this.

   33  [Test]

   34         public void a_single_value_is_wrapped_in_curly_braces()

   35         {

   36             testEnumerable = new List<String> {"ABC"};

   37             expectedResults = "{ABC}";

   38         }


easy enough, right? 


Here's how I made it pass.


   15  ///

   16         /// Retuns a joined string with "and" as the last join.

   17         ///

   18         ///

   19         public static string ToNonOxfordCommaString(this IEnumerable<String> enumberable)

   20         {

   21            var joinedValue = GetJoinedValue(enumberable);

   22            return String.Format("{{{0}}}",joinedValue);

   23         }

   24 

   25         private static string GetJoinedValue(IEnumerable<string> enumberable)

   26         {

   27             var totalCount = enumberable.Count();

   29 

   30             switch (totalCount)

   31             {

   32                 case 0:

   33                     return String.Empty;

   34 

   35                 case 1:

   36                     return enumberable.ToArray()[totalCount - 1];

   37              }

   38             return null;

   39         }


Simple, and works.


Next, I need to handle the "and" join... Here's the unit test.


   39  [Test]

   40         public void two_values_uses_and_to_join()

   41         {

   42             testEnumerable = new List<String> {"ABC", "DEF"};

   43             expectedResults = "{ABC and DEF}";

   44         }


This looks simple enough, we're just gunna add a "default" case, and use and as the join. I know this won't work for more than 2 values.... Once again, I refuse to write more code than needed to make a unit test pass. Here it is.


   25 private static string GetJoinedValue(IEnumerable<string> enumberable)

   26         {

   27             var totalCount = enumberable.Count();

   29 

   30             switch (totalCount)

   31             {

   32                 case 0:

   33                     return String.Empty;

   34 

   35                 case 1:

   36                     return enumberable.ToArray()[totalCount - 1];

   37                 default:

   38                     return String.Join(" and ",

   39                                        enumberable.ToArray()

   40                                );

   41 

   42             }

   43         }


Yep, all green unit tests... Next pass is to handle the most complex senario. 


   45 [Test]

   46         public void more_than_2_values_use_commas_with_and_as_the_last_value()

   47         {

   48             testEnumerable = new List<String> { "ABC", "DEF", "G", "H" };

   49             expectedResults = "{ABC,DEF,G and H}";

   50         }


This is one (of the many) reasons I love TDD... I'm pretty sure at this point that the most complex senario only needs one small modification in GetJoinedValue to work. We'll just get the last value, and use LINQ IEnumerable extensions to get all values except the last one, and add the last one to it. By not worrying about the complex stuff at first, I've made it easy.


Here's the (changed) code.

   25  private static string GetJoinedValue(IEnumerable<string> enumberable)

   26         {

   27             var totalCount = enumberable.Count();

   28             var lastValue = enumberable.ToArray()[totalCount - 1];

   29 

   30             switch (totalCount)

   31             {

   32                 case 0:

   33                     return String.Empty;

   34 

   35                 case 1:

   36                     return lastValue;

   37                 default:

   38                     return String.Join(",",

   39                                        enumberable.Take(totalCount - 1).ToArray()

   40                                ) + " and " + lastValue;

   41 

   42             }

   43         }



Vola! Every unit test passes, and requirements are met, and 100% tested. 


Wait... I think I see a bug! (Actually, it was Nick who saw it, thanks!). What if there are no sequences in the list, the line 


   28 var lastValue = enumberable.ToArray()[totalCount - 1];



Would throw an ArgumentOutOfRange exception.


When Nick pointed this out, I wrote the following unit test.


   51 [Test]

   52         public void no_sequences_does_not_throw_argument_out_of_range()

   53         {

   54             testEnumerable = new List<String>();

   55             expectedResults = "{}";

   56         }



Yep, it fails. Easy enough to fix, right? 


   25 private static string GetJoinedValue(IEnumerable<string> enumberable)

   26         {

   27             var totalCount = enumberable.Count();

   28             var lastValue = totalCount == 0 ?

   29                 String.Empty:

   30                 enumberable.ToArray()[totalCount - 1];

   31 

   32             switch (totalCount)

   33             {

   34                 case 0:

   35                 case 1:

   36                     return lastValue;

   37                 default:

   38                     return String.Join(",",

   39                                        enumberable.Take(totalCount - 1).ToArray()

   40                                ) + " and " + lastValue;

   41 

   42             }

   43         }


While these requirements wheren't horribly complicated, I think this example shows how nicely TDD lends itself to progressive ehancements and by extension makes the most complex requirements easier.


Challange for all my (5) readers...


Fix this bad boy to use polymorphism, so we don't violate OCP.

2 comments:

  1. Sheesh! It's about time you got back on the blog scene! You kept telling me about all of this fitnesse research you were doing. I'd love to hear the results ;)

    I'm too lazy to write the code up but I'm guessing we replace the switch and turn that into a factory that returns classes (that extend some sequence list formatting interface) that know what to do with the list of sequences. I couldn't tell if there was any desire to continue to use extension methods. If not, then build some other object that orchestrates the string formatting.

    And I dunno if you're being honest with the 5 of us...at the start of the GetJoinedValue method, doesn't this code throw an index out of range exception when there are no sequences in the list?

    var totalCount = enumberable.Count();
    var lastValue = enumberable.ToArray()[totalCount - 1];

    ReplyDelete
  2. Thanks Nick, Edited the post to include writing a unit test that reproduces the bug you mentioned and fixes it.

    Yeah, Ideally, that's the way I'd like to handle it (Factory). I was just trying to get you guys write some code on my blog! :)

    ReplyDelete