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...