Getting NUnit to go all STA

As I was trying to help out someone who had trouble with writing a unit test for a data bound WPF Window.. first of all I had to ref WindowsBase, PresentationCore and PresentationFramework and then I ran into a curt warning from WPF when you're trying to instantiate a WPF window in a NUnit test case.. (Grumble Grumble... I hate UI in unit tests)
TestProj.TestBindings.TestTextBoxBinding:System.InvalidOperationException : The calling thread must be STA, because many UI components require this.
What is STA? Something you only wish 'stays out of the way'.
Interesting.. this means... its not running in STA. Elementary. Now how do I get NUnit to go STA. It was a long arduous road.
First up if you have NUnit 2.5 or above, I believe its as easy as this.

[Test, RequiresMTA]
public void MTA()
{
   Assert.AreEqual(ApartmentState.MTA, Thread.CurrentThread.ApartmentState);
}

[Test, RequiresSTA]
public void STA()
{
   Assert.AreEqual(ApartmentState.STA, Thread.CurrentThread.ApartmentState);
}

If not, which is where I was. It's slightly more complicated.
First go to

C:\Program Files\NUnit 2.4.8\bin

Look for a file called NUnitTests.config. Next copy this over to your test dll folder which has TestProj.nunit (or make one.. you'll need it). Rename the copy of the file to 'TestProj.config'.. Open it up in an editor.. Time for adding bringing some more XML into the world.

<configuration>
  <configSections>
    <sectionGroup name="NUnit">
      <section name="TestRunner" type="System.Configuration.NameValueSectionHandler"/>
    </sectionGroup>
  </configSections>

  <NUnit>
    <TestRunner>
      <add key="ApartmentState" value="STA"/>
    </TestRunner>
  </NUnit>
  <!-- all the other fluff stays -->
</configuration>

That's it, fire up NUnit.. if all goes well, you'll be running in STA and WPF will 'show itself'.

Graphs in Ruby On Rails

Well I was building my personal expense tracking web app and soon enough I need to graph the top expense categories... A little searching led to gruff and it worked right outta the box.

First up, Gruff needs RMagick which needs ImageMagick. Tricky.. so go here and I hit Q#7
Soon you should land up here download the rmagick-win32 gem zip. This archive has both RMagick gem and the install for the right version of ImageMagick. Installing the latest version of ImageMagick may not work.. (I didnt try.. the docs warned me off).
So install ImageMagick first. Next install the gem with 'gem install rmagick --local' in the folder where you unzipped the archive.
Now switch to your Rails Dev Environment. Open up config/environment.rb - Add this line at the end 'require gruff'
Add a new action to the controller of your choice.

  def showTopN
g = Gruff::Bar.new
g.title = "Hello Gruff"
g.theme_37signals

Expense.get_top_n_categories(:limit=>10).each{ |category|
g.data(category.name, category.total_amount.to_f)
}

send_data g.to_blob, :disposition=>'inline', :type=>'image/png', :filename=>'top_n.pdf'
end

Now close all command windows. Restart Webrick (your Rails Web Server) to take into account changes made to your PATH env var.
Fire up a browser.. in my case to http://localhost:3000/outflow/showTopN

Update:Normal people would be overjoyed at this point.. but not me. I wanted to create this graph at runtime .. So when I click on a link that I imagine will be there, I callback on my controller (yes using AJAX), create my graph and then slot it into a div tag on the same page. This will help me chart different sub-sets of data without leaving the page.
But how? so I post a question on stackoverflow
Solution:
report_controller.rb

require 'gruff'
class ReportController < ApplicationController
 def showTopNCategories

 end

 def drawBarChart
g = Gruff::Bar.new
     g.title = "Hello Gruff"
     g.theme_37signals
  
     Expense.get_top_n_categories(:limit=>10).each{ |category|
         g.data(category.name, category.total_amount.to_f)
     }
     g.write('public/images/top_n.png')
     render(:text=>'top_n.png')
 end
end

showTopNCategories.rhtml

<%content_for("page_scripts") do -%>
function updateImg(id, imageFile)
{
 $(id).innerHTML = '<img src="/images/' + imageFile + '"/>';
}
<% end -%>

<!-- some controls -->
<%= link_to_remote("Refresh Chart",
 :complete => "updateImg('div_barchart', request.responseText)",
 :url=>{:action=>'drawBarChart'})%>

<div id="div_barchart">

</div>

application.rhtml

<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>
<!-- the usual -->

<script type="text/javascript"> <%= @content_for_page_scripts %> </script>
</head>
<!-- rest of the usual -->

The quirk here was that

• gruff considers the Rails Project folder as the base folder.. so you need public/images/FileName.png as the parameter to g.write. Rails views however have public as the base folder. So image paths are relative.. /images/FileName.png
• the javascript function didn't wire up correctly. It dies silently if you make a typo for instance. Use alert("Hit!"); liberally to know if the function is being hit.
  

Optional parameters in Ruby

Well I keep forgetting this. There's a way to pass in a hash of optional parameters as the last parameter to a method.. how do I do that? I'll save you some time. Here's how.

  def Expense.get_top_n_categories options={}
sQuery = "SELECT categories.name, sum(amount) as total_amount
  from expenses
  join categories on category_id = categories.id
  group by category_id
  order by total_amount desc";
sQuery += " limit #{options[:limit].to_i}" if !options[:limit].nil?
Expense.find_by_sql(sQuery)
end

# Now you see a param.. top 10  
obCategories = Expense.get_top_n_categories :limit => 10

# now you don't
obCategories = Expense.get_top_n_categories

The cleverness lies in defaulting the options param to an empty hash. Now you can pass any number of params using the param => value notation.. everything comes neatly packaged to you as an options hash.

How did you manage to get the code formatted so pretty? http://blog.wolfman.com/articles/2006/05/26/howto-format-ruby-code-for-blogs

ShowMeTheMoney-16 Let's play tag the expense with a category

Well I know its been a long time... I took a break.. I got married in it. Long story short... its been a while. But now let's end what I started
Basically we want to tag each expense with some text. Not much time would be spent on the updating and deleting categories. If we can just create categories and select existing ones, we should be good. We’ll need to get some confirmation from the user on this.. So we’ll defer the delete for sometime.
So basically a dropdown with existing categories – that is editable. So while entering the expense entry, the user can either choose an existing category or add a new one.





I wrote up an acceptance test. Although the former is more readable (being a DoFixture.. however I find that FitLibrary isn't ported yet for Ruby) So we have to make do with the corresponding ActionFixture.. more verbose but will do.

First lets add a migration to add the categories table and add a foreign key in the expenses table to refer to the former.
> ruby script/generate migration add_categories
This will create the corresponding file in db\migrate folder.

class AddCategories < ActiveRecord::Migration
def self.up
create_table :categories do |table|
table.column :name, :string, :limit => 80
end
execute "INSERT INTO categories VALUES (1, 'General');"
add_column :expenses, :category_id, :integer, {:null=>false, :default =>1}
execute "alter table expenses add constraint fk_expenses_to_categories foreign key(category_id) references categories(id)"

end

def self.down

execute "ALTER TABLE expenses DROP FOREIGN KEY fk_expenses_to_categories"
remove_column :expenses, :category_id
drop_table :categories

end
end

Now to run this migration against all three databases dev, prod and test
> rake db:migrate RAILS_ENV="test"


Add the category model with the unit tests for that one. We focus only on creation and retrieval .. don’t see any edits or deletes happening for categories

class CategoryTest < Test::Unit::TestCase
def test_create …
def test_saveWithoutCategoryName_fails ….
end

Now onto expense_test.rb
We need to

• create the expense category if it does not exist
• use the existing category if possible
• ensure that the associated category is retrieved from the plot

the first two should be the responsibility of the outflow_controller
create categories.yml first. update expense model tests one at a time to account for the category.
Update OutflowController to create or use-existing category on creation of expense entry.

class OutflowController < ApplicationController
...

auto_complete_for :category, :name

def create
raise ArgumentError, "Category missing!", caller[1..-1]  if (params[:category].nil? || params[:category][:name].nil?)  

sCategoryName = (params[:category][:name]).strip
category = Category.find(:first, :conditions=>["name = ?", sCategoryName])
category = Category.create( {:name => sCategoryName} ) if (category.nil?)
params[:expense][:category_id] = category.id

@expense = Expense.new(params[:expense])
if (@expense.save)
 flash[:notice] = 'Expense entry saved'
 redirect_to :action=>'list'
else
 render :action=>'new'
end
end
end

Update views to have an auto-completing field for the user to enter the category for an expense. This is a one-line addition in the controller as shown below coupled with the following in the view.
/app/views/outflow/_form.rhtml

<tr>
<td> <%= getAppString(:label_category) %> </td>
<td>
<%= text_field_with_auto_complete :category, :name%>
</td>
</tr>

Right on! Let’s complete the implementation required for the second part of our acceptance test.

class ExpenseGetTopNCategoriesTest < Test::Unit::TestCase
fixtures :expenses, :categories

def test_getTopN_categories
 obCategories = Expense.get_top_n_categories

 assert_equal 3, obCategories.length
 assert_equal categories(:rent_category).name, obCategories[0].name
 assert_in_delta expenses(:rent_expense_entry).amount, obCategories[0].total_amount, 0.01

 assert_equal categories(:apparel_category).name, obCategories[1].name
 assert_in_delta expenses(:apparel_expense_entry).amount, obCategories[1].total_amount, 0.01

 assert_equal categories(:entertainment_category).name, obCategories[2].name
 assert_in_delta expenses(:movies_expense_entry).amount + expenses(:another_movie_expense_entry).amount,
     obCategories[2].total_amount, 0.01
end

...

run ruby script/server –s
a little playing around helps me settle on this query

Expense.find_by_sql("SELECT categories.name, sum(amount) as total_amount
from expenses
join categories on category_id = categories.id
group by category_id
order by total_amount desc")

Stackoverflow users help me to convert this into Rails Model speak

Expense.find(:all, :select =>

"categories.name name, sum(amount)
total_amount",
     :joins => "inner join categories on category_id =
categories.id",
     :group => "category_id",
     :order => "total_amount desc"

That turns out fine. I add more tests and a couple of more tables to the acceptance test to retrieve top n categories, top categories in a date range, top n categories in a date range. Soon enough...



Check in everything. Repeat the same for tagging inflows with categories. Should be easy. Until next time...

IDE-Ninja Shortcuts with VS2008 and CodeRush Express

Just some nifty IDE tricks that I picked up from the PDC talk by Dustin Campbell at PDC2008
First off

Go to Definition is F12, Find all references is Shift+F12. Nothing new here.. but lets say you did F12 multiple times and lost your way in the jungle of code. Don’t fear.. Shift+Ctrl+8 backtracks through each step you took to take you back to the starting point. Shift+Ctrl+7 cycles forward again in case you want to get lost again :)
Useful : Sometimes

Lets say you have multiple files open in your IDE. Now you know you want to switch to an open file.. To do so like an IDE Ninja.. Press Ctrl+Alt+Down key-combo will bring to up the following popup



You can now start typing if you know the initial characters of the file name and press Enter to switch.
Useful : Quite

Next up, have you been trying to zoom in on those tiny squiggles, right click and choose a refactoring or some other action.



That tiny thing is called a Smart Tag and now you can activate it by doing the Ctrl+. (dot) combo to invoke the context menu when the cursor is under the word. You don't need to leave the keyboard to apply a refactoring or implement an interface ever again.
Useful : Very

CODERUSH : For the next set of moves, you need to install CodeRush Express which DevExpress + Microsoft have graciously partnered to give away to VS2008 users for free. 34 Meg download. Express Edition users.. tough luck!

Jump to File: Once you have CodeRush installed and a solution open. Use the uber Ctrl+Alt+F combo to bring up the following popup



Type in the filename, use arrow keys to select, press Enter and Voila! Insta-Teleport. And yes it does substring matches too.. you can type in part of the filename like 'Coffee'. Sweet!
Useful : Very

Can I do the same for jumping to methods? I remember I wrote a test for warmer plate status.. what was it called? Shift+Ctrl+Q for



I type in some characters and the results narrow down. Ah! there it is. Enter and teleport again. This shortcut is to Jump to Symbol.. so it works for member variables, properties, etc too.
Useful : Sometimes

Finally we have Put some GPS trackers on this variable. Ctrl+Alt+U



This will highlight all places where the symbol is used, you can then use tab to navigate between each reference. Esc to make these trackers disappear.
Useful : Rarely.. when you're in machete mode in someone else's code.

That's all folks!

My solution for UncleBob's Mark IV CoffeeMaker - Retrospective

Back to Part4?

Part 5: Rear-view mirror.

So it's the end of the flashback.. for anyone who lasted this far. I've the design all coded out and it works. So let's have a look at the design that we ended up with. Might be a good side-by-side compare with the first post in this series.
Final touch-ups that I did,

• renamed SensorObserver to ObservableCoffeeMakerDevice.. long name but it is an internal class so there.
  
• Also I renamed some event-handler in CoffeeMaker in the name of readability and all that is good. On_WarmerPlateStatusChangedAfterBrew >> TurnIndicatorLightOn_AsSoonAsThereIsFreshCoffee.
  

Since we have our safety-net of auto-tests, we can do it with disdain.. Piece of cake.



These are the Object cubes (flattened) for the final set of classes. Contrast with the objects in Post 1. So what did I learn?

• A little DUF helped cut down the number of classes I started off with.. Sketching them out on paper, helped me filter out lazy classes. I think I did JED here.. Just enough design to get moving.. good move.
  
• I'm a little sloppy / not thorough with my TDD; I missed out some tests and then went out of my way to add them as and when I remembered. Maybe spending some time to list out all the tests.. just the test-method names as empty stubs could be a counter-measure
• Felt uneasy for a while while I was building a SensorObserver component (in full) one test at a time without completing any user-stories. Maybe I should have implemented just enough of SensorObserver to implement a story#1..
  
• I conveniently forgot about the Brew Button in my initial design sketches.. However it didn't prove to be a big time-sink of a omission.
• A major chunk of my time was spent debugging a multi-threaded issue. TDD didn't help..Console.WriteLines saved the day. After that it was smooth sailing.
• Object Cubes aid in thinking about design.. I'm definitely adding this to my toolkit (Although I could do without Side 3.. A simple + (public) and -(private) before the method name helped me depict the public and private contracts)
• Object Thinking the book shunned "controller" classes. I couldn't think of a way to come up with a CoffeeMaker design that doesn't control the device via the exposed API.
  
• This is a very nice OOD problem.. deceivingly simple at first sight..worth burning some brain cells on. Now I need to finish the rest of UncleBob's APPnP book.
  

Food for thought: I wonder how different the design would emerge if someone just started off and continued one test-at-a-time... without any design up-front?

Want the 69kb source code zip (built onVS2008 Express Edition)?.. drop me a mail at gishu[dot]pillai[at]gmail[dot]com or let me know of an easy way to place this zip on an online shelf.

My solution for UncleBob's Mark IV CoffeeMaker - 4

Back to Part3?

Part 4: Fresh Coffee at the end of this post!

Story#2. The Warmer Plate is turned on only when the pot is present on it.
Going back to my lil seq diagrams, we see that this one is similar to the previous one. We let the WarmerPlateStatus changed event tell us when the pot has been moved off the plate and toggle the heater on/off accordingly.

[Test]
public void Test_WarmerHeaterIsTurnedOff_WhenPotWithCoffeeIsTakenOffThePlate()
{
m_obDevice.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;
Assert.AreEqual(WarmerState.ON, m_obDevice.WarmerPlateState,
  "Warmer Plate should be ON if there's a pot with cofee on it");

m_obDevice.WarmerPlateStatus = WarmerPlateStatus.WARMER_EMPTY;
Assert.AreEqual(WarmerState.OFF, m_obDevice.WarmerPlateState,
  "Warmer Plate should be turned off");
}

And to make that go Green

void On_WarmerPlateStatusChanged(object sender, EventArgs obEvtArgs)
{
...
if (WarmerPlateStatus.POT_NOT_EMPTY == m_obDevice.GetWarmerPlateStatus())
{
  m_obDevice.SetWarmerState(WarmerState.ON);
}
else
{
  m_obDevice.SetWarmerState(WarmerState.OFF);
}
}

Easy enough.. complete the rest.

• Test_WarmerPlateStaysOff_WhenEmptyPotIsReplacedOnPlate
  
• Test_WarmerPlateIsTurnedOn_WhenEmptyPotStartsCollectingCoffee
  
• Test_WarmerPlateStaysOff_WhenEmptyPotIsTakenFromThePlate
  
• Test_WarmerPlateIsTurnedOn_WhenPotWithCoffeeIsReplacedOnPlate
  

Story#3: When the Brew Button is pressed, the CoffeeMaker makes Coffee.
The description is in the book. The user places the filter with coffee grounds, pours water and presses the BrewButton. The indicator light goes off till Coffee is ready. This just seems like too big a bite..
The CoffeeMaker’s BrewButton is an auto-resetting button. So we add that into FakeCoffeeMaker.

public BrewButtonStatus GetBrewButtonStatus()
{
BrewButtonStatus eBrewButtonStatus = m_eBrewButtonStatus;
m_eBrewButtonStatus = BrewButtonStatus.NOT_PUSHED;
return eBrewButtonStatus;
}

The test for this isn’t flowing to me. I’ll look at the rough seq diagram I drew up.. Ah! the huge seq diagram is scaring me.. I need to break this diagram into multiple tests

1. Test_BrewButtonPressed
2. Test_BrewButtonPressed_ButTheUserForgotToPourWater
3. Test_IndicatorLightIsTurnedON_AsSoonAsCoffeeIsReady
4. Test_BoilerIsTurnedOFF_WhenWaterIsExhausted

[Test]
public void Test_BrewButtonPressed()
{
Assert.AreEqual(BoilerState.OFF, m_obFakeDevice.BoilerState);
Assert.AreEqual(IndicatorState.ON, m_obFakeDevice.IndicatorState);

m_obFakeDevice.SimulateUser_PlaceFilter_PourWater_And_PressTheBrewButton();

Assert.AreEqual(BoilerState.ON, m_obFakeDevice.BoilerState,
  "Boiler must be turned on to heat the water");
Assert.AreEqual(IndicatorState.OFF, m_obFakeDevice.IndicatorState,
  "Indicator light must go off to indicate CoffeeMaker is busy");
}

So to make this compile I need to update FakeCoffeeMaker with (I know its an awfully big method name..)

internal void SimulateUser_PlaceFilter_PourWater_And_PressTheBrewButton()
{
m_eBoilerStatus = BoilerStatus.NOT_EMTPY;
PressBrewButton();
}

Alright we have a red to go ahead.. So first the CoffeeMaker has to know when the BrewButton is pressed. The Observer can help us with that.

public void Monitor(CoffeeMakerAPI obDevice)
{
...
m_obObserver.WarmerPlateStatusChanged += new EventHandler(On_WarmerPlateStatusChanged);
m_obObserver.BrewButtonPressed += new EventHandler(On_BrewButtonPressed);

On_WarmerPlateStatusChanged(this, EventArgs.Empty);
}

void On_BrewButtonPressed(object sender, EventArgs e)
{
m_obDevice.SetBoilerState(BoilerState.ON);
m_obDevice.SetIndicatorState(IndicatorState.OFF);
}

Green. Next we handle the error scenarios. The user can forget the filter (and I presume he may get hot water.. not too bad.. I’ll skip that one). But if he forgets to add water, we don’t want our boiler all heated up for nothing. So

[Test]
public void Test_BrewButtonPressed_ButTheUserForgotToPourWater()
{
Assert.AreEqual(BoilerState.OFF, m_obFakeDevice.BoilerState);
Assert.AreEqual(IndicatorState.ON, m_obFakeDevice.IndicatorState);

m_obFakeDevice.SimulateUser_PlaceFilter_And_PressTheBrewButton();

Assert.AreEqual(BoilerState.OFF, m_obFakeDevice.BoilerState,
  "Boiler must stays off since there is no water in the boiler");
Assert.AreEqual(IndicatorState.ON, m_obFakeDevice.IndicatorState,
  "Indicator light stays as is as CoffeeMaker refuses to work without water");
}

Now I can add another method to this thing, which leaves out the ‘Pour Water’ part. But something doesn’t feel right. I might as well as kill off the Lazy Method & have separate methods for each user action. Like this.

m_obFakeDevice.SimulateUser_PlaceFilterWithCoffeeGrounds();
m_obFakeDevice.SimulateUser_PressTheBrewButton();

I get squiggly lines and I use my newfound IDE ninja knowledge ‘Ctrl + . + Enter’ to generate the stub methods. Then Ctrl+Alt+DownArrow + F(akeCoffeeMaker) + Enter to switch over to the class.

internal void SimulateUser_PlaceFilterWithCoffeeGrounds()
{
return;
}

internal void SimulateUser_PressTheBrewButton()
{
return;
}

Now I see we already have a PressBrewButton… that we can rename. Delete the new stub#2. Stub#1 can stay as is.. just to add to readability of the test. Build and good.. red test.
Simple fix.. add a guard clause and we’re green.

void On_BrewButtonPressed(object sender, EventArgs e)
{
if (m_obDevice.GetBoilerStatus() == BoilerStatus.EMPTY)
{
  return;
}
...

We’ll update the previous test to be consistent and readable as follows. So the previous test now reads...

[Test]
public void Test_BrewButtonPressed()
{
...
m_obFakeDevice.SimulateUser_PlaceFilterWithCoffeeGrounds();
m_obFakeDevice.SimulateUser_PoursWater();
m_obFakeDevice.SimulateUser_PressTheBrewButton();
...
}

Next up is

[Test]
public void Test_IndicatorLightIsTurnedON_AsSoonAsCoffeeIsReady()
{
Test_BrewButtonPressed();

m_obFakeDevice.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;

Assert.AreEqual(IndicatorState.ON, m_obFakeDevice.IndicatorState,
  "Indicator light comes back on - 'Coffee Ready'");
}

And red straight away.. Make the following changes to Coffee_Maker.Brew

void On_BrewButtonPressed(object sender, EventArgs e)
{
...

m_obDevice.SetBoilerState(BoilerState.ON);
m_obDevice.SetIndicatorState(IndicatorState.OFF);

m_obObserver.WarmerPlateStatusChanged += new EventHandler(On_WarmerPlateStatusChangedAfterBrew);
}

void On_WarmerPlateStatusChangedAfterBrew(object sender, EventArgs e)
{
if (m_obDevice.GetWarmerPlateStatus() != WarmerPlateStatus.POT_NOT_EMPTY)
{   return; }
m_obDevice.SetIndicatorState(IndicatorState.ON);
m_obObserver.WarmerPlateStatusChanged -= new EventHandler(On_WarmerPlateStatusChangedAfterBrew);

}

Neat. Now on to the final one.

[Test]
public void Test_BoilerIsTurnedOFF_WhenWaterIsExhausted()
{
Test_BrewButtonPressed();

m_obFakeDevice.SetBoilerStatus(BoilerStatus.EMPTY);

Assert.AreEqual(BoilerState.OFF, m_obFakeDevice.BoilerState,
  "Boiler should have been turned off since there is no more water to heat");
}

Red.. Back to Coffee_Maker

void On_BrewButtonPressed(object sender, EventArgs e)
{
...

m_obDevice.SetBoilerState(BoilerState.ON);
m_obObserver.BoilerEmpty += new EventHandler(On_BoilerEmpty);
m_obDevice.SetIndicatorState(IndicatorState.OFF);
m_obObserver.WarmerPlateStatusChanged += new EventHandler(On_WarmerPlateStatusChangedAfterBrew);
}

void On_BoilerEmpty(object sender, EventArgs e)
{
m_obDevice.SetBoilerState(BoilerState.OFF);
m_obObserver.BoilerEmpty -= new EventHandler(On_BoilerEmpty);
}

All Green. Now I update my visual CoffeeMakerView that I built to strap on a Coffee_Maker to monitor the FakeCoffeeMakerDevice and see the magic happen. I need to make some of the internal test helper FakeCoffeeMaker methods public. Warm Fuzzy Glow! It is alive!!! [CoffeeMakerTestApp\bin\Release\CoffeeMakerTestApp.exe]



Thought of one more test.. Ready Indicator doesn't stays OFF if the pot already had some old coffee in it. Add a new test and make it green by only turning off the light and subscribing for notifications if we have an empty pot on the plate. Done!
Turn the page..

My solution for UncleBob's Mark IV CoffeeMaker - 3

Back to Part 2?

Part 3: Beware of threads

The CoffeeMaker opens the Relief valve in case someone takes the coffee pot off the plate to prevent coffee from falling onto the plate
From the seq dia #2,the CoffeeMaker just needs to listen for changed events from the WarmerPlate.
If new status is WarmerEmpty, open the valve to reduce pressure/stop water flow. Else keep it closed.

[TestFixture]
public class TestCoffee_Maker
{
 private const int I_WAIT_FOR_EVENTS_TO_FIRE = 300;
 [Test]
 public void Test_ValveIsOpened_IfCoffeePotIsRemoved_ElseClosed()
 {
     Coffee_Maker obCoffeeMaker = new Coffee_Maker();
     FakeCoffeeMakerDevice obDevice = new FakeCoffeeMakerDevice();
     obCoffeeMaker.Monitor(obDevice);
     obDevice.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;
     Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

     Assert.AreEqual(ReliefValveState.CLOSED, obDevice.ReliefValveState,
         "Relief valve should be closed when Pot With Coffee is on the plate");
     obDevice.WarmerPlateStatus = WarmerPlateStatus.WARMER_EMPTY;
     Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

     Assert.AreEqual(ReliefValveState.OPEN, obDevice.ReliefValveState,
         "Relief valve should be open since the warmer plate is now empty");

 }
}

Red! make it pass like this.

public class Coffee_Maker
{
 SensorObserver m_obObserver;
 CoffeeMakerAPI m_obDevice;

 public void Monitor(CoffeeMakerAPI obDevice)
 {
     m_obObserver = new SensorObserver();
     m_obDevice = obDevice;

     m_obObserver.Observe(m_obDevice);
     m_obObserver.WarmerPlateStatusChanged += new EventHandler(On_WarmerPlateStatusChanged);

 }

 void On_WarmerPlateStatusChanged(object sender, EventArgs obEvtArgs)
 {
     if (WarmerPlateStatus.WARMER_EMPTY == m_obDevice.GetWarmerPlateStatus())
     {
         m_obDevice.SetReliefValveState(ReliefValveState.OPEN);
     }
 }
}

Next we extend to check if it is closed when the pot is placed back on it. On second thoughts.. let me write 4 test cases for each transition… be thorough. One at at time.. Improve the name of the first one

public void Test_ValveIsOpened_IfPotWithCoffeeIsRemovedFromThePlate() {..}
[Test]
public void Test_ValveIsClosed_IfPotWithCoffeeIsReplaced ()
{
 Coffee_Maker obCoffeeMaker = new Coffee_Maker();
 FakeCoffeeMakerDevice obDevice = new FakeCoffeeMakerDevice();
 obCoffeeMaker.Monitor(obDevice);
 obDevice.WarmerPlateStatus = WarmerPlateStatus.WARMER_EMPTY;
 Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

 Assert.AreEqual(ReliefValveState.OPEN, obDevice.ReliefValveState,
     "Relief valve should be open since the warmer plate is now empty");
 obDevice.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;
 Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

 Assert.AreEqual(ReliefValveState.CLOSED, obDevice.ReliefValveState,
     "Relief valve should be closed when Pot With Coffee is on the plate");
}

Make it green by

void On_WarmerPlateStatusChanged(object sender, EventArgs obEvtArgs)
{
 if (WarmerPlateStatus.WARMER_EMPTY == m_obDevice.GetWarmerPlateStatus())
 {
     m_obDevice.SetReliefValveState(ReliefValveState.OPEN);
 }
 else
 {
     m_obDevice.SetReliefValveState(ReliefValveState.CLOSED);
 }
}

Still red ??!! aha.. hit upon a special case. If the coffee maker is started with no pot on the plate, the valve should be open. Since no ‘change’ occurs.. event is never fired. We can fix that by calling the above function at the end of Monitor() to initialize the valve correctly.. Done
Refactoring time: Move common code to a Setup method and extract local variables to members
The second test fails intermittently… bad omen. Is it because two threads are accessing the same variable.? think the variable needs to be marked as volatile. Still no luck.
Looks like my TDD badge is going to be revoked. I put in some consoles traces to see if the event is being raised.

A detour into 'Thrashing around in a multi-threaded environment'
I find that when the test fails, the LastPlateStatus is WARMER_EMPTY to begin with.. hence the event is never raised. The consoles/traces are:
Start Listener thread
Change to WE
Listener thread active
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY

And when it works
Start Listener thread
Listener thread active
Check POT_EMPTY with POT_EMPTY
Change to WE
Check WARMER_EMPTY with POT_EMPTY
WARMER_EMPTY <= POT_EMPTY Check WARMER_EMPTY with WARMER_EMPTY Check WARMER_EMPTY with WARMER_EMPTY Check POT_NOT_EMPTY with WARMER_EMPTY POT_NOT_EMPTY <= WARMER_EMPTY Check POT_NOT_EMPTY with POT_NOT_EMPTY Check POT_NOT_EMPTY with POT_NOT_EMPTY

Batman to the drawing board. I trace out the seq of events. (Unnecessary in hind sight..I can see that the issue is clear from the above traces).
Aha! In the RED Case, The listener thread starts after the WarmerPlateStatus has been changed to WE.. i.e. the listener thread is slow off the blocks.
From my doodling, I see that it’d be better if I ensure that the listener thread is started before the SensorObserver.Observe method exits. Let’s see if that is the problem.

public void Observe(CoffeeMakerAPI obDevice)
{
 m_obDevice = obDevice;
 obListenerThread = new Thread(new ThreadStart(ListenForChanges));
 obListenerThread.Start();

 WaitTillListenerThreadHasStarted();
 Console.WriteLine("Start Listener thread ");
}
private void WaitTillListenerThreadHasStarted()
{
 while (obListenerThread.ThreadState != ThreadState.Running) ;
}

private void ListenForChanges()
{
 Console.WriteLine("Listener thread active");
 ...

Aaarghh!! Still intermittent red. When in doubt.. I add more traces.. (expert TDDer at work here.. NOT)

public void Observe(CoffeeMakerAPI obDevice)
{
 m_obDevice = obDevice;
 obListenerThread = new Thread(new ThreadStart(ListenForChanges));
 obListenerThread.Start();

 Console.WriteLine("Start Listener thread ");
 WaitTillListenerThreadIsRunning();
 Console.WriteLine("Out of Observe!");
}

private void ListenForChanges()
{
 Console.WriteLine("Listener thread active");
 BoilerStatus eLastBoilerStatus = m_obDevice.GetBoilerStatus();
 WarmerPlateStatus eLastPlateStatus = m_obDevice.GetWarmerPlateStatus();
 Console.WriteLine("First Status Read");
 while ( !m_bStopWatching )
 {
 …

Now when it fails.. the log is
Start Listener thread
Out of Observe!
Listener thread active
Change to WE
First Status Read
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY
Check WARMER_EMPTY with WARMER_EMPTY

Aha! The thread has started but just before it could initialize the LastXXX status variables, there is a thread switch and the value is changed to WE. If we promote the LastXXX variables to volatile local variables and ensure that they are always initialized before the listener thread is started.. hmm. lets see if this works…
Hallelujah! Console.WriteLines rock()

private volatile BoilerStatus m_eLastBoilerStatus;
private volatile WarmerPlateStatus m_eLastPlateStatus;

public void Observe(CoffeeMakerAPI obDevice)
{
 m_obDevice = obDevice;
 m_eLastBoilerStatus = m_obDevice.GetBoilerStatus();
 m_eLastPlateStatus = m_obDevice.GetWarmerPlateStatus();
 Console.WriteLine("First Status Read");
 obListenerThread = new Thread(new ThreadStart(ListenForChanges));
 obListenerThread.Start();

 Console.WriteLine("Start Listener thread ");
 Console.WriteLine("Out of Observe!");
}

private void ListenForChanges()
{
 Console.WriteLine("Listener thread active");
 while ( !m_bStopWatching )
 {
 …

Now the logs show.
First Status Read
Start Listener thread
Out of Observe!
Listener thread active
Check POT_EMPTY with POT_EMPTY
Change to WE
Check WARMER_EMPTY with POT_EMPTY
WARMER_EMPTY <= POT_EMPTY

Turn the crank another 20 times.. All green and good. I’ll remove all the logging lines.. Goodbye friends!
More duplication I see that we have repeating occurrences of
m_obDevice.Property = value
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
How about we move the delay into the FakeCoffeeMaker property.. then we don’t need to define the constant in each test fixture.

public class FakeCoffeeMakerDevice : CoffeeMakerAPI
{
 private const int I_WAIT_FOR_EVENTS_TO_FIRE = 300;

 public WarmerPlateStatus WarmerPlateStatus
 {
     get { return m_eWarmerPlateStatus; }
     set
     {
         m_eWarmerPlateStatus = value;
         Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
     }
 }

Look how pretty my tests are now.. Act-Arrange-Assert appears again to knight ‘em as SirGoodTest.

[Test]
public void Test_ValveIsOpen_IfPotWithCoffeeIsRemovedFromThePlate()
{
 m_obDevice.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;
 Assert.AreEqual(ReliefValveState.CLOSED, m_obDevice.ReliefValveState,
     "Relief valve should be closed when Pot With Coffee is on the plate");

 m_obDevice.WarmerPlateStatus = WarmerPlateStatus.WARMER_EMPTY;

 Assert.AreEqual(ReliefValveState.OPEN, m_obDevice.ReliefValveState,
     "Relief valve should be open since the warmer plate is now empty");
}

Watch in awe ... haphazard refactoring ninja on steroids
Repeat the procedure for TestSensorObserver fixture.. add delays to PressBrewButton() and SetBoilerStatus(). Cool Green!
Exposed! The Missing Dispose! Hey we’re missing SensorObserver.Dispose() in the third test. I should have mistake-proofed that by adding a Teardown method that does that.. never too late.

Which brings me to… I’m not doing SensorObserver.Dispose() in our TestCoffeeMaker tests. Lets add a Dispose to CoffeeMaker that delegates the call and call it from Teardown. Green!
I’ll just add two tests to complete the set.

• Test_ValveIsOpened_IfPotWithCoffeeIsTakenOffThePlate
• Test_ValveIsClosed_IfPotWithCoffeeIsReplaced
• Test_ValveIsOpened_IfEmptyPotIsTakenOffThePlate
• Test_ValveIsClosed_IfEmptyPotIsReplacedOnPlate

what about the transition PE => PNE (The pot starts collecting coffee). Lets add that one too

• Test_ValveIsClosed_IfEmptyPotIsReplacedOnPlate

Turn the page...

My solution for UncleBob's Mark IV CoffeeMaker - 2

Back to Part1?

Part 2: Watch those sensors

From my little design exercise, it looks like we need someone who will poll the sensors periodically and alert us with events.
I have already created an implementation of the CoffeeMakerAPI interface called FakeCoffeeMakerDevice that we'll use for our task.
Test first.. here we go

[TestFixture]
public class TestSensorObserver
{

private int m_iNotificationCount;
[SetUp]
public void Setup()
{
m_bEventRaised = 0;
}

[Test]
public void Test_BrewButtonPressedEvent_IsRaised()
{
SensorObserver obObserver = new SensorObserver();
FakeCoffeeMakerDevice obCoffeeMaker = new FakeCoffeeMakerDevice();
obObserver.Observe(obCoffeeMaker);
obObserver.BrewButtonPressed += new EventHandler(On_Event_Raised);

obCoffeeMaker.PressBrewButton();
Thread.Sleep(300);

obObserver.Dispose();
Assert.IsTrue(m_bEventRaised, "BrewButton Pressed But SensorObserver didnt notify
us");
}

private void On_Event_Raised(object sender, EventArgs e)
{
m_bEventRaised = true;
}

Now the code to make this pass turned out to be a bit longwinded. TDD with multithreading right off the bat.

public class SensorObserver
{
private CoffeeMakerAPI m_obDevice;

public event EventHandler BrewButtonPressed;

private bool m_bStopWatching = false;
private Thread obListenerThread;

private bool m_bIsDisposed = false;

public void Observe(CoffeeMakerAPI obDevice)
{
m_obDevice = obDevice;
obListenerThread = new Thread(new ThreadStart(ListenForChanges));
obListenerThread.Start();
}

private void ListenForChanges()
{
while ( !m_bStopWatching )
{
  if (BrewButtonStatus.PUSHED == m_obDevice.GetBrewButtonStatus())
  {
      if (null != this.BrewButtonPressed)
      {
          this.BrewButtonPressed(this, EventArgs.Empty);
      }
  }

  Thread.Sleep(100);
}
}

public void Dispose()
{
if (m_bIsDisposed)
{    return;        }
m_bStopWatching = true;
obListenerThread.Join(2000);
m_bIsDisposed = true;
}
}

• Add a similar test for BoilerEmpty event. Done.
• Refactor common test code into setup and teardown.
• Remove the magic numbers related to delays for threads

[TestFixture]
public class TestSensorObserver
{
SensorObserver obObserver;
FakeCoffeeMakerDevice obCoffeeMaker;
private int m_iNotificationCount;

const int I_WAIT_FOR_EVENTS_TO_FIRE = 300;

[SetUp]
public void Setup()
{
   m_iNotificationCount = 0;

   obObserver = new SensorObserver();
   obCoffeeMaker = new FakeCoffeeMakerDevice();
   obObserver.Observe(obCoffeeMaker);
}

[Test]
public void Test_BrewButtonPressedEvent_IsRaised()
{
   obObserver.BrewButtonPressed += new EventHandler(On_Event_Raised);

   obCoffeeMaker.PressBrewButton();
   Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

   obObserver.Dispose();
   Assert.IsTrue(m_iNotificationCount != 0, "BrewButton Pressed But SensorObserver didnt notify us");
}
...

But now it seems that the Sensor Observer would nag us with events till we put some water in the boiler. We want only one notification when the boiler goes from NOT_EMPTY to EMPTY. (Not a problem with Brew Button since the spec says that the CoffeeMaker auto-resets it to NOT_PUSHED.)
Looks like we need a test to verify that only one event is raised. We could write a new test but that would be the same as the previous one except for the end assertion. How about if we use an integer value instead of a boolean to check for event notifications.. we could then check m_iNotification == 1 to verify that the event was raised.. and only once.
Turns out I need to add another delay before setting BoilerState to EMPTY in the test else the change is too fast for SensorObserver to see.

[Test]
public void Test_BoilerEmptyEvent_IsRaised_ExactlyOnce()
{
obCoffeeMaker.SetBoilerStatus(BoilerStatus.NOT_EMTPY);
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
obObserver.BoilerEmpty += new EventHandler(On_Event_Raised);

obCoffeeMaker.SetBoilerStatus(BoilerStatus.EMPTY);
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);

obObserver.Dispose();
Assert.IsTrue(m_iNotificationCount == 1, "Boiler has run out of water but SensorObserver has notified us {0} times", m_iNotificationCount);
}

Code to make that green:

private void ListenForChanges()
{
BoilerStatus eLastBoilerStatus = m_obDevice.GetBoilerStatus();
while ( !m_bStopWatching )
{
   ...

   BoilerStatus eCurrentBoilerStatus = m_obDevice.GetBoilerStatus();
   if ((BoilerStatus.EMPTY == eCurrentBoilerStatus) && (eCurrentBoilerStatus != eLastBoilerStatus))
   {
       if (null != this.BoilerEmpty)
       {
           this.BoilerEmpty(this, EventArgs.Empty);
       }
   }
   eLastBoilerStatus = eCurrentBoilerStatus;
   Thread.Sleep(FOR_100_MILLISEC);
}

Great. Next we need a notification which lets us know of changes in the WarmerPlateStatus… and I‘m beginning to feel like a BDUFish (building up a component without a user story).. but this is the last event .. I’ll trudge on.
The WP can be in 3 states.. let me draw up a quick state diagram.
Shows that there are 5 valid transitions.. (Invalid Transitions: PotNotEmpty ->PotEmpty. PotEmpty->WarmerEmpty is suspect but I’ll let it slide.. (Wally just checked if there’s any… and snuck it back.. The puppet master shall lie in wait).
Hmm as I think about it.. I just need a changed notification.. the new value can be queried.. Let see if we can do with a simple ‘changed’ event..

[Test]
public void Test_WarmerPlateStatusChanged()
{
obCoffeeMaker.WarmerPlateStatus = WarmerPlateStatus.POT_EMPTY;
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
obObserver.WarmerPlateStatusChanged += new EventHandler(On_Event_Raised);

obCoffeeMaker.WarmerPlateStatus = WarmerPlateStatus.POT_NOT_EMPTY;
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
Assert.IsTrue(m_iNotificationCount == 1, "Sensor Observer missed the Warmer Plate PE->PNE transition");

obCoffeeMaker.WarmerPlateStatus = WarmerPlateStatus.WARMER_EMPTY;
Thread.Sleep(I_WAIT_FOR_EVENTS_TO_FIRE);
Assert.IsTrue(m_iNotificationCount == 2, "Sensor Observer missed the Warmer Plate PNE->WE transition");

// and so on for all 6 transitions

Code is similar to that of BoilerEmpty event except that you raise an event whenever there is a change between last and current values
The test and the ListenForChanges method both are nearly exceeding a screenful.. but I’ll let it be for now. I don’t see more additions coming.. if they do I‘ll slice-n-extract methods.

Ok so now we have everything ready for the real deal.. CoffeeMaker. Lets begin with something simple.
Turn the page...

My solution for UncleBob's Mark IV CoffeeMaker

Part 1: et tu BDUFus?

What is the Mark IV CoffeeMaker? It's an OOD problem from Bob Martin's Agile Principles, Patterns and Practices in C#.
Well how hard could it be... actually quite a bit...a bout of designers' block. I thrashed about for a while on paper and drew up the following diagrams to start up with.. which fit the bill.


Fig: The brew coffee seq dia.


Fig: L - Keep Coffee Warm, R - Control Relief valve

Some more thinking, I landed up with the following object cubes (from ObjectThinking.. lets see if this helps me). To summarize,

1. Side1: Classic CRC card
2. Side2: Description + applicable stereotypes if any
3. Side3: Contracts (N.A. this early in the design process.. usually private, public, protected, ISP derived interfaces etc.
4. Side4: Knowledge required by the class to fulfill its responsibility (Either a [V]ariable, [A]rgument in service request, from [C]ollaborator or [M]anufacture on demand.)
5. Side5: Protocol / Methods
6. Side6: Events

Fig: Object cubes - first cut.
Note:CMA - CoffeeMakerAPI implementation from spec. Also it should be marked as (C) instead of (V) in Side4.. my mistake.

But wait.. flashes of clarity.

• IndicatorLight and Valve are see-thru delegators to CMA and don't do any real work. A tell-tale sign is that they do not have any methods that they claim as their own. Eliminate them.
  
• et tu Boiler? The only thing Boiler seems to have against being axed on the same lines.. is an if clause to prevent starting the boiler if there is no water. Off with its head.. I'll move that logic into CoffeeMaker
  
• What about CoffeeMaker?.. it has the Brew method. This is sort of a Controller class that Object Thinking warns us against... a puppet master for the device.. but we'll live with it for the sake of this series of posts ;)
  
• The x_Sensor classes are way too similar except that they watch 2 different status values. I can club them into a single class.. SensorObserver (a bad name in hindsight.. its more of an ObservableCoffeeMakerDevice)
  

So now we have Revision#2




Good so we're down to 2 classes from the original 7 (excluding CMA). That was time well spent.. or is it? The only way to validate if this design is good.. Code it up.

Turn the page...

Object Thinking

The book. Author: David West.

Remember Alan Kay's quotes that 'Object revolution is yet to come' & 'C++ wasn't what I had in mind when I coined the term object-oriented'? Now I'm no fanboy so I set about objectively looking at the matter presented in this book if there's any meat behind the argument 'You're all not getting it'. I read this book twice.. the second time trying to distill ideas that are innovative or new or unknown to me before I read this one.

There are n solutions to a single problem. All of them right in solving the problem however only a few extend the existing system at a 'natural joint' i.e. where it is consistent with the original structure/intent or theory of the developer. Such changes lead to systems that are easy to take-down and reassemble when you hit a dead end.. (along the lines of the Lego Brick metaphor, if you find that a brick doesn't give you the shape you want, you just chuck the brick and plug in a different shape.) This in fact is the bulb moment behind the XP practice of 'Metaphor'. Identifying the right set of objects and the distribution of responsibilities among them to suimulate a real world scenario is the sign of a master object designer. This book will not make you an artist but will give you some good pointers nonetheless.

• The first few chapters help the reader understand between the culture difference between 2 camps of 'scientific logical method-driven designers' and 'heuristics-driven artistic social designers'.
  
• Chapter 3 outlines the 4 fundamental tenets of object oriented thinking.
  
• Chapters 4 n 5 throw some light on the terminology and subtle thinking differences.
  
• Chapter 6,7,8,9 are the 'How-to'/actionable chapters.
  
• Chapter 10 is just bizarre... disjointed
  

The book as a whole doesn't exhibit 'flow'... something feels off.
The book definitely could have been shorter to make a better impact- I liked a bunch of ideas like Object Cubes (an extension to the CRC Card) as a thinking tool (I don't think I'll be building any physical cubes), the 4 presuppositions of object thinking, 'All inheritance must be based on behavior extensions', the little 'Behind The Quotes' sidebars , 'Object as a person' metaphor. Self-evaluating rules, Event Dispatchers and the DataItem type (as opposed to passive data) are interesting. That said it is a refreshing change to read about the craft behind OOP. 4 out of 5 stars

Who moved my metaphor?

Recently I posted a question on StackOverflow (Plug Plug) regarding 'what is a metaphor in Extreme Programming XP' ? I got a few answers but wasn't placated. It seemed (to me) to be the secret door in XP, which separates the masters from the wannabes. The other practices are relatively transparent.. easy to pickup.. but this one is elusive.. open to interpretation. So I dug up some books that I have and went about seeking the truth

Let's start with something simple.

Ron Jeffries: Metaphor is a supporting practice in XP. A metaphor or System of Names provides a common language for talking about the design of the program - helps communicate quickly and effectively. e.g. 'This system is like an assembly line for checks instead of cars'

Pretty simple.. but is that all? Let's see what the master has to say in XP Explained: Embrace Change
Kent Beck: Each XP Project is guided by a single overarching metaphor. Helps everyone understand the basic elements and their relationships. The words used to identify technical entities should be consistently taken from the chosen metaphor. Metaphor in XP replaces much of what is normally termed 'architecture'. Calling a 10000 meter view of the system the 'architecture'.. doesn't push the system into any sense of cohesion. The goal of architecture should be to give everyone a coherent story within which to work, a story that can be easily shared by business and technical folks. Asking for a metaphor is more likely to provide this.
You can start development armed with 'just a metaphor' as long as

• you have concrete feedback from real code and tests validating if the metaphor is working in practice
• the customer is comfortable talking in terms of the metaphor
• you refactor to continually refine your understanding of what the metaphor means in practice.
  

Hmm... the third bullet is a bit mystic for me.. but let's move on.However in the second edition of the book, Metaphor has been knocked off its pedestal as an XP practice. Passingly mentioned...
Interaction designers on an XP team choose an overall metaphor for the system. Conventional interaction design mandates a phased approach.. however this should be avoided as phases reduce feedback and restrict flow of value.
Finally in TDD By Example, as part of the retrospective for Part 1.
I have programmed money (around 20 times). Then while writing this, I thought of using expression as the metaphor and the design went in a completely different direction. I really didn't expect metaphor to be so powerful. A metaphor should be just a source of names, shouldn't it.? Apparently not. The metaphor Ward C. used for 'several monies together with potentially different currencies' was a mathematical vector, I've used MoneySum, MoneyBag and finally Wallet, which imply merging 2 values with same currency. The expression metaphor freed from this, the code came out cleaner and clearer than ever before.
Seems like Mr. Beck specifically spoke about its importance @ OOPSLA 2002.. titled 'The Metaphor Metaphor' where he offered to finally 'give it up' if a show of hands confirmed that metaphor isn't working in practice. I searched for a while... but the transcript isn't anywhere to be found. All I could find was some notes from a human memory.. couldn't grasp much.. maybe 'you had to be there' (OR could it be that I was missing the metaphor :)

Let's dig into the matter a bit more... this time we follow 'software development's archaelogist cum anthropologist'.
Alistair Cockburn: discuss the value of XP's metaphor-setting exercise, we need to step back to a piece called 'Programming as Theory Building" (1985)
Peter Naur: Programming should be correctly regarded as an activity by which programmers form or achieve a certain kind of insight, a theory, of the matters at hand. This is in stark contrast of the common notion of programming as production of a program and other texts. This is the 'Theory Building View' of programming.
Example: Group A built a compiler for language L. Later, Group B had the task of building a compiler for a language L+M, a modest extension. GroupB worked out a contract with GroupA - terms of support : full documentation, including annotated program text (Src code with comments?), additional written design docs and personal advice. GroupB went through the design phase and formed suggestions on how to accommodate the extensions & submitted it to GroupA for review.
GroupA found instances where the proposed solutions made no use of the facilities inherent in the existing structure of the compiler & which were discussed at great length in the documentation. Instead they were patches or 'grafted on' effectively destroying its power and simplicity. They were able to spot these cases instantly and propose simple and effective alternate solutions framed entirely within the existing structure.
A few years down the line, GroupC took over the reins for the compiler, without any guidance from GroupA . A member of the original A-Team later commented that the structure though still visible, had been rendered ineffective by varied amorphous additions.
Point: Full Source code + additional Documentation is insufficient in conveying the theory to another (motivated) group of programmers.
Mr. Naur does another backreference to the work of Ryle(1949) and Kuhn (1970)
Theory = knowledge a person must have in order to not only do certain things intelligently but also to explain them, to answer queries and debate, etc. The notion is not confined to what may be the called the general/abstract part of the insight. e.g. For a person to have Newton's theory of mechanics doesn't equate to just knowing the central laws such as F = m a. He/she must have an understanding of the manner in which the central laws apply to certain aspects of reality. e.g. must understand how the theory of mechanics applies to motion of pendulums and planets and must be able to recognize similar phenomena in the world to be able to employ the theory properly. The dependence of theory on a grasp of certain kinds of similarity between situations and events of the real world gives the reason why the knowledge held by a person who has the theory cannot be expressed in terms of rules. (e.g. Newton's laws)
A programmer who has built a theory

1. Can explain how the solution relates to the affairs of the real world that it helps to handle. Conversely, for any aspect or activity of the world the programmer is able to state its manner of mapping into the program text.
2. Can explain why each part of the program is what it is.. is able to support the actual program text with a justification based on the programmers' direct intuitive knowledge.
3. Able to respond constructively to any demand for modification of the program to support new behavior in the world. Can perceive any similarity between the new demand and the operational facilities already built into the program (if it exists).
   

When faced with a change request (maintenance), one hopes to achieve cost savings by making modifications to an existing program rather than writing one from scratch. The metaphor of civil construction in this regard doesn't hold true. Secondly the expectation may be based on the premise that the program is a text held in a medium capable of being edited easily.
Similarly, the idea of including flexibility into a program - we build in operation facilities that are not immediately demanded but which are likely to be useful. However this comes at substantial cost of design-implementation-test-description, incurred for a feature whose usefulness depends entirely on future events.
The theory building view debunks both of the above. When faced with a change, first of all, what is needed is a confrontation of existing solution with the demands called for by the desired modification. The degree and kind of similarity has to be determined.. which brings out the merit of the theory building view. The decay of program text as a result of modifications made by programmers without a proper grasp of the the underlying theory now is understandable. If viewed merely as a modification of program text and of external behavior of execution, any given modification can be realized in different ways, all correct. But if viewed with the perspective of the theory behind the program (only possible for a programmer who has built the theory), these ways may look very difficult.. some conforming to it and extending it in natural ways while others may be wholly inconsistent. To retain the program's quality, simplicity, good structure, etc. it is mandatory that each modification is firmly grounded in the theory of it.
The death of a program happens when the programmer team possessing its theory is dissolved. The program itself may continue to be executed by a computer and produce useful results. The actual state of death becomes visible when demands for program modifications cannot be intelligently answered.
Oh! you're still reading.!... just a little bit more.
Alistair Cockburn: Viewing programming as "theory building" helps us understand "metaphor building" in XP. Kent suggested that it's useful to a design team to simplify the general design of a program to a single metaphor e.g. assembly line, restaurant etc.
If a metaphor is good, the many associations the designer creates around the metaphor turn out to be appropriate to their programming situation. (Naur's idea of passing along a theory of the design). Later programmers will make guesses about the structure of the software (based on their knowledge of 'assembly lines' for instance) and find that their guesses are "close". That is an extraordinary power for just 2 words. The value of a good metaphor increases with the number of people working in parallel. The closer each designer's guess is "close" to other designers, the greater the resulting consistency of the final design. In its absence, each will develops his/her own theory with time. As each one one contributes code to the pool, the theory loses coherence. Not only does maintenance become difficult, development also is harder. The design becomes a 'kludge'. An appropriate shared metaphor lets a person guess where someone else on the team just added code, and how to fit her new piece in with it.
The designers are allowed to use multiple metaphors if they don't find one adequate for the entire system. e.g. this piece is like people waiting at the bank, this piece is like a school bus, etc.

Are we there yet?
David West: Some would claim all thinking is metaphor-based. Metaphors are essential for bridging the familiar and the unfamiliar; selection of the "right" metaphors is critical for further development of fundamental ideas (discovery) . Every story/test/code module employs labels and terms that metaphorically connect things in the domain with their simulated coded counterparts.

So now do I have my answers?

• What is a metaphor?
  

1. A System of Names, common language shared by customers and developers.
2. Guides discovery of objects, responsibilities, relationships, etc. for a coherent software solution
3. Helps Peter Naur's 'theory building' and passing it on to future developers
4. Instrumental for keeping the evolving design coherent and preserve theory in the face of multiple designers working on discrete pieces in parallel.

• So how do I identify a metaphor for the System under development?

Look at how entities interact in the real world ecosystem that is (or part of which is) being modelled as software. Correllate it with similar smaller simpler systems in the real world that display self-similarity with the ecosystem. (This looks like a bee-hive/ant-colony except...) Run with it till the metaphor breaks or is inconsistent with some real world observation.. then change to a better metaphor.

• How do I know its a good one? What makes a good metaphor good? edit-clarification: Can a bad metaphor do more harm than good? What does it enable?

The 'goodness of a metaphor' concept has been described by Alistair Cockburn's excerpt above.
Bad (or relatively worse) metaphors exist and can lead to mediocre designs
e.g. Consider David West's example of finding a metaphor for an object in software. If your metaphor for objects is something like this image, you'd come up with a UML-ish design with static class diagrams and relationships. However if your metaphor for an object is a person and use that to guide your design, you're bound to have a good design with fewer classes that fully communicate intent.

1. People don't like to be told how to work.. you should ask them to do something and provide them with the information needed & expect them to do a good job. It's possible to insert wires into your head and make you dance by sending you the right electric impulses to make you dance... but that doesn't mean someone should do it. Don't build X_controller and X_manager classes whose only job is to tell other objects how to do something.
2. People are lazy.. if something is too complex for them to do, they enlist the help of a specialist. Don't create large objects. Distribute responsibilities to cohesive classes and let them collaborate to perform the complex task.
3. Each person may be different. Avoid type explosion..Don't create class hierarchies based on data/attributes. A bluebird is just a Bird object with an additional :color = blue entry in its NamedValueCollection member.
   
4. and so on...
   

• How do I get good at metaphor finding... or better than I am currently?

I'm afraid I haven't found an answer to this.. As Peter Naur said '(to further talent for theory formation) The most hopeful approach is to have the student work on concrete problems under guidance, in an active and constructive environment.'

Thanks for reading! I've paraphrased most of the statements above and included examples as per my understanding. You'd be well advised to actually read the following references for more details..

References: Get your own copy. Highly recommended.
Kent Beck * Extreme Programming Explained : Embrace Change First & Second Edition * Test Driven Development By Example
Alistair Cockburn * Agile Software Development : the co-operative game
Ron Jeffries * Extreme Programming Adventures in C#
David West * Object Thinking