Tuesday, 17 August 2010

Total WTF moment whilst reading Clean Code

I've just spent a useful and enjoyable day finally getting around to reading Clean Code, definitely a book to come back to time and again for reference, etc.  However given the subject matter the book contains I was amazed to find in the section about "magic numbers" a statement that "some constants are so easy to recognise that they don't always need a named constant".  Two of the examples given are the number of feet per mile and the number of work hours per day.   I'm not sure how many people that work with imperial units could easily state how many feet are in a mile.  If you are more used to using metric units then that figure has next to no meaning what so ever.  And at least in the UK working hours vary depending upon the industry, company and even department that you work in.  Whilst it doesn't detract from the content of the book I'm not sure those statements should have got past the proof reading / draft review stage.

Monday, 16 August 2010

Response to "Two different ways to create bad logic in unit tests"

Today I read Roy Osherove's blog post "Two Different ways to create bad logic in unit tests".  It's an interesting article that covers logic included in many unit tests, is it acceptable to compare a returned string value using a comparison value built via string concatenation?  It is perfectly possible that an issue introduced by the concatenation process is duplicated in both the code and the unit test - more often than not the logic has been cut / pasted from the code base into the unit test anyway.  

As an example directly from Roy's blog consider the following code:

   1: [TestMethod]

   2: public void SomeTest()

   3: {

   4:   string user = "user";

   5:   ComponentUnderTest cut = new ComponentUnderTest();

   6:   string result = cut.SayHello(user);

   7:   string expected = "hello " + user;

   8:   Assert.AreEqual(expected,result);

   9: }

Thinking this through, it makes perfect sense, the following could be a better test:

   1: [TestMethod]

   2: public void SomeTest()

   3: {

   4:    string user = "user";

   5:    ComponentUnderTest cut = new ComponentUnderTest();

   6:    string result = cut.SayHello(user);

   7:    string stringPrefix = "Hello ";

   8:    Assert.IsTrue(result.StartsWith(stringPrefix, result);

   9:    Assert.IsTrue(result.EndsWith(string.Format(" {0}", user), result);

  10:    Assert.AreEqual(stringPrefix.Length + user.Length, result.Length);

  11: }

Could this be the answer?  It certainly removes any possible duplicated logic that may hide issues in the string manulipation, but would it be workable in a more complex example?

Sunday, 15 August 2010

Project Euler – Problems 18 & 67

The challenge set by problem 18 was

By starting at the top of the triangle below and moving to adjacent numbers on the row below, the maximum total from top to bottom is 23.

7 4
2 4 6
8 5 9 3

That is, 3 + 7 + 4 + 9 = 23.

A 15 row triangle was then supplied for which the program must determine the corresponding maximum value taking a similar path through the data.  A foot note to the problem highlighted that due to the “limited” number of paths for this puzzle it could be solved using brute force determining the total for each path through the triangle and selecting the maximum total returned.  However a link to problem 67 was provided which exactly the same problem, but this time the data was for a hundred row triangle. For that problem a brute force attack could not be used as the number of possible routes meant that:

If you could check one trillion routes every second it would take over twenty billion years to check them all. There is an efficient algorithm to solve it. ;o)

In researching possible approaches I came across an Excel solution by Tushar Metha.  This really simplified the approach, turning the data upside down and starting on the last row, taking two adjacent cells (“c1” and “c2”) and their corresponding cell on the row below “c3” (remember triangle has been turned upside down).  The two routes for that subset were determined and the maximum value was placed in cell “c3”.  In the example, 5 is “c1”, 9 is “c2” and 4 is “c3”.  So “c1 + c3” = 9 whilst “c2 +c3” = 13, so the contents of c3 is replaced with 13.  This process is repeated for the entire row. 

5   9


Once the entire row has processed you “move” down a row and repeat the process.  For problem 67, instead of taking 20 billion years it takes less than a second - so quite a nice time saving over the brute force approach.

Source code: VS2010 C# Solution

Problem Stats:

  • 23 out of 299 problems solved (2 more until first level!)
  • No Ranking (based on problems 275 to 299)
del.icio.us Tags: ,

Monday, 9 August 2010

Project Euler – Problem 54

The challenge set by Problem 54 was to determine the number poker games payer 1 won given a text file detailing 1,000 hands dealt to 2 players.  Given the logical nature of the rules, the solution was just a case of finding the best way to 1) implement the rules and 2) duplicate the rule hierarchy.  I quickly re-factored my first attempt that attempted to place the ruled based logic inside of the PokerHand class as I found whilst this made it easy to compare the matching combinations of a single hand it was much hard to compare two hands and determine the winning combination – this was made even harder when two hands contained the same winning combination and the next highest combination had to be selected from each to be compared.

At this point I simplified the PokerHand class to just being a container for an IEnumerable Card collection and some basic logic to confirm the hand was valid (i.e. contained 5 cards).  The logic to find the winning player was migrated into a PokerEngine class, which took two populated instances of PokerHand (player 1 and player 2).  After some basic validation, both hands are passed into a rule which checks to see which hand (if any) wins.  If neither hand wins or draw, then the next rule in hierarchy is checked, until either a matching hand is found, or the round is deemed a draw (as the documentation clearly stated that each hand had a clear winner this situation would result in an exception being thrown).  To help until testing and the re-factoring of the original code, each validation rule was still performed against a single hand, returning the outcome of the check.  The outcome of the checks performed against both hands were used to determine a winner.

The following code is the first logic check to find the winning hand, the first combination to check for is the Royal Flush as it beats any other.  If both hands contain a Royal Flush, a quick short short cut can be called to see which hand has the highest card

var winningPlayer = DetermineIfWinOnRoyalFlush(player1Hand, player2Hand);
if (winningPlayer != WinningHandEnum.None)
return winningPlayer;

The above code calls the function shown below.  This function utilises the separate rule hander that checks hands in isolation.  This means each method on HandValidationRules can be unit tested in isolation of each other.

  1. private static WinningHandEnum DetermineIfWinOnRoyalFlush(PokerHand player1Hand, PokerHand player2Hand)
  2. {
  3.     var player1Pair = HandValidationRules.RoyalFlush(player1Hand);
  4.     var player2Pair = HandValidationRules.RoyalFlush(player2Hand);
  6.     if (player1Pair == null && player2Pair == null)
  7.         return WinningHandEnum.None;
  9.     if (player1Pair == null)
  10.         return WinningHandEnum.Player2;
  11.     if (player2Pair == null)
  12.         return WinningHandEnum.Player1;
  14.     return WinningHandEnum.None;
  15. }

The validation rules for RoyalFlush are shown below:

internal static RoyalFlushWinningCombination RoyalFlush(PokerHand pokerHander)
if (!pokerHander.ValidHand)
return null;
if (ValueOfHighestCard(pokerHander) != 14)
return null;
if (!AllOfSameSuite(pokerHander) || !ConsecutiveValues(pokerHander))
return null;
return new RoyalFlushWinningCombination();

$(Document) is null or not an object

I've just spent a very painful hour or so debugging a jQuery issue that turned out to be a self inflicted problem!  The site I was working on had been working perfectly all morning, then a particular page began to fail on a refresh (browser F5).  It was possible to browse to the page and it displayed correctly, but press F5 to refresh the page and it failed on the statement below.  Checking the contents of document showed it to be populated, but $(document) failed with an error saying it was either null or not an object.

$(document).ready(function() {

It just didn't make sense, going to the page everything would work as expected but refreshing the very same page caused it to fail - it didn't matter if it was the start up page of the visual studio project or not.  In one of the many attempts to isolate / identify the problem I cleared out the browser cache.  Amusingly this now broke the entire site - the error occurred every time the page was viewed.

............It was at this point that I noticed I'd accidently moved the projects localised copy of jQuery.  The requests to the page that had worked originally had been using a browser cached copy of the file - it was the refresh of the page that was acting correctly.