GSharper

Friday 30 March 2007

Conditional dispatcher to Command Pattern

Some time ago, a customer asked me for a solution for the following problem:

They had to send messages to certain people. These messages where kept in a database table. The messages contained variables which had to be filled in by the person who sends the message. For example:

I will be [NumberInput] minutes late.

You need to make a delivery to address [AddressList].

Please call telephone number [TelephoneList].

When the operator selects the first message, a textbox is shown where he has to input a delay.

When the second message is selected, a combobox will be shown containing addresses.

When the third message is selected, a listbox will be shown containing telephone numbers.

So I created a little application which retrieved the messages from the database in a Combobox. When selecting one of the messages, the following code runs

Label lblTitle;
if (messageSelector.SelectedItem.ToString().IndexOf("NumberInput") != -1)
{
Label lblTitle
= new Label();
lblTitle.Text
= "Input Number"
messagePanel.Controls.Clear();
TextBox txtNumberInput
= new TextBox();
txtNumberInput.Name
= "NumberInput"
messagePanel.Controls.Add(lblTitle);
txtNumberInput.Width
= 120;
txtNumberInput.Location
= new Point(100, 0);
messagePanel.Controls.Add(txtNumberInput);
}
else if (messageSelector.SelectedItem.ToString().IndexOf("AddressList") != -1)
{
Label lblTitle
= new Label();
lblTitle.Text
= "Choose Address"
messagePanel.Controls.Clear();
ComboBox cmbAdressList
= new ComboBox();
cmbAdressList.Name
= "AddressList"
// Get addresses from the database.
GetAddresses(cmbAdressList);
messagePanel.Controls.Add(lblTitle);
cmbAdressList.Width
= 120;
cmbAdressList.Location
= new Point(100, 0);
messagePanel.Controls.Add(cmbAdressList);
}
else if (messageSelector.SelectedItem.ToString().IndexOf("TelephoneList") != -1)
{
Label lblTitle
= new Label();
lblTitle.Text
= "Choose Telephone"
messagePanel.Controls.Clear();
ListBox lstTelephoneList
= new ListBox();
lstTelephoneList.Name
= "TelephoneList"
// Get telephone numbers from the database.
GetTelephoneNumbers(lstTelephoneList);
messagePanel.Controls.Add(lblTitle);
lstTelephoneList.Width
= 120;
lstTelephoneList.Location
= new Point(100, 0);
messagePanel.Controls.Add(lstTelephoneList);
}

This is definitely a violation of the open-closed principle. And duplicate code? A lot of it.

But the customer wanted a quick solution and they said no other messages will be added with different controls. One week later, a new message was added :)


Please call telephone number [TelephoneInput].


This message resulted in a special textbox GsmNumberTextBox which did some extra validations using regular expressions.


So, I could add another conditional statement but I was sure that there would be additional messages in the future. So time to change some stuff.


I read an article, "Replace conditional dispatcher with command pattern".


First I would do the refactor step "Extract Method":


if (messageSelector.SelectedItem.ToString().IndexOf("NumberInput") != -1)
{
ShowNumberInput();
}
else if (...)

private Label ShowNumberInput()
{
Label lblTitle;
lblTitle
= new Label();
lblTitle.Text
= "Input Number"
messagePanel.Controls.Clear();
TextBox txtNumberInput
= new TextBox();
txtNumberInput.Name
= "NumberInput"
messagePanel.Controls.Add(lblTitle);
txtNumberInput.Width
= 120;
txtNumberInput.Location
= new Point(100, 0);
messagePanel.Controls.Add(txtNumberInput);
return lblTitle;
}

I do the same for the other conditional steps.

The next refactor step is "Extract class":



class NumberInputTextBox
{
TextBox _txtNumberInput;

public NumberInputTextBox()
{
_txtNumberInput
= new TextBox();
}

public Control CreateNumberInput()
{
Control control
= new UserControl();
control.Width
= 276;
Label lblTitle;
lblTitle
= new Label();
lblTitle.Text
= "Input Number"
_txtNumberInput.Name
= "NumberInput"
control.Controls.Add(lblTitle);
_txtNumberInput.Width
= 120;
_txtNumberInput.Location
= new Point(100, 0);
control.Controls.Add(_txtNumberInput);
return control;
}

public string Text
{
get { return _txtNumberInput.Text; }
set { _txtNumberInput.Text = value;}
}
}

I create a NumberInputTextBox and move all the code from the ShowNumberInput method to a public class method CreateNumberInput. The Text property has to be exposed so I create a public property. In the CreateNumberInput method I create some sort of placeholder named control which I decorate with the necessary controls.


I do the same for the other conditional steps. So now we have three classes.


Now I change the conditional steps to call the new classes and I also remove the call to the old ShowNumberInput method.



if (messageSelector.SelectedItem.ToString().IndexOf("NumberInput") != -1)
{
messagePanel.Controls.Clear();

NumberInputTextBox numberInputTextBox
=
new NumberInputTextBox();

messagePanel.Controls.Add
(numberInputTextBox.CreateNumberInput());
}
else if (...)

I apply the step "Extract Interface" on the classes. I have to decide a common command execution method name. Now we have :



  • CreateNumberInput
  • CreateAddressListBoxCombo
  • CreateTelephoneListBox

Let's change these names to CreateParameterControl and create the interface:



public interface IHandler
{
Control CreateParameterControl();
}

Let the parameter classes implement IHandler. Now every class' Create... method is renamed to CreateParameterControl.

The next step is to change the calls in the conditional statements to the new CreateParameterControl method.


if (messageSelector.SelectedItem.ToString().IndexOf("NumberInput") != -1)
{
messagePanel.Controls.Clear();

NumberInputTextBox numberInputTextBox
= new
NumberInputTextBox();

messagePanel.Controls.Add(numberInputTextBox.CreateParameterControl());
}


Now we have a couple of different solutions to go further with the refactoring:



  • Create a Command map and fill it with key value pairs being the name of the parameter to create and the resulting parameter class.
  • Create a Factory which uses reflection (see my article Reflection instances ) to let a config file decide which parameter requires which control.

First let's see how the the Command map solution works:


First of all we create a map. I'll use a generic dictionary for it like this:



IDictionary<string,Control> _paramCreationMap;
During initialization we fill the map:


private void CreateHandlerMap()
{
_paramCreationMap
= new Dictionary<string,Control>();
_paramCreationMap.Add(
"NumberInput", new NumberInputTextBox().CreateParameterControl());
_paramCreationMap.Add(
"AddressList", new AddressListComboBox().CreateParameterControl());
_paramCreationMap.Add(
"TelephoneList", new TelephoneListBox().CreateParameterControl());
}

the previous conditional dispatcher, can now be completely removed and replaced with this code:


// Get the param string out of the selectedItem using reflection
string paramName = Regex.Match(messageSelector.SelectedItem.ToString(), @"(?<=\[).*(?=\])").Value;
messagePanel.Controls.Clear();
Control paramControl
= _paramCreationMap[paramName];
messagePanel.Controls.Add(paramControl);

So no more conditional if...then...else...else if... or switch case statements.


Now, lets look at the second solution:


The result I'm trying to achieve is to get rid of the filling of a map with instances. Because when a new control is created, the instance has to be added to the map which I don't particularly like.


Then, a friend of mine showed me the beauty of reflection. I created my reflection based factory like this:



public static class ParameterFactory
{
public static Control CreateParameter(string paramName)
{
string assemblyName =
ConfigurationManager.AppSettings[
"ParameterControlsProject"];

string typeToLoad =
ConfigurationManager.AppSettings[paramName];

ObjectHandle objectHandle
=
Activator.CreateInstance(assemblyName,typeToLoad);

IHandler parameter
= (IHandler)objectHandle.Unwrap();
Control control
= parameter.CreateParameterControl();
control.Name
= typeToLoad;

return control;
}
}

The factory gets called like this:



// Get the param string out of the selectedItem using reflection
string paramName =
Regex.Match(messageSelector.SelectedItem.ToString(),
@"(?<=\[).*(?=\])").Value;

messagePanel.Controls.Clear();

Control paramControl
= ParameterFactory.CreateParameter(paramName);

messagePanel.Controls.Add(paramControl);

In the config file, in the AppSettings section we declare some key-value pairs which specify, which paramName corresponds with which type to load:




<appSettings>
<add key="ParameterControlsProject" value ="ParameterControls"></add>
<add key="NumberInput" value ="ParameterControls.NumberInputTextBox"></add>
<add key="AddressList" value ="ParameterControls.AddressListComboBox"></add>
<add key="TelephoneList" value ="ParameterControls.TelephoneListBox"></add>
</appSettings>


The key corresponds with paramName. ParameterControlsProject resembles the project where the actual control classes reside. Maybe we can change this to get the name from the calling assembly or something. I'll have to try this out.


That's it,


Greetz, G

Labels: , ,

0 Comments:

Post a Comment

Subscribe to Post Comments [Atom]



<< Home