The first thing to observe is that the five pieces of code are identical except for two parameters that are varying. We can capture the common the code in a function (let's call it populateComboBox
), and call that five times:
populateComboBox $UI.Combobox1 $ColumnsHashTable.Column1
populateComboBox $UI.Combobox2 $ColumnsHashTable.Column2
populateComboBox $UI.Combobox3 $ColumnsHashTable.Column3
populateComboBox $UI.Combobox4 $ColumnsHashTable.Column4
populateComboBox $UI.Combobox5 $ColumnsHashTable.Column5
function populateComboBox($comboBox, $column)
{
For($i = 0; $i -lt $column.Length; $i++) {
$comboBox.Items.Add($column[$i])
}
}
It's best not to use old C-style for
loops with an index unless you need to because they add visual noise:
populateComboBox $UI.Combobox1 $ColumnsHashTable.Column1
populateComboBox $UI.Combobox2 $ColumnsHashTable.Column2
populateComboBox $UI.Combobox3 $ColumnsHashTable.Column3
populateComboBox $UI.Combobox4 $ColumnsHashTable.Column4
populateComboBox $UI.Combobox5 $ColumnsHashTable.Column5
function populateComboBox($comboBox, $column)
{
foreach ($item in $column)
{
$comboBox.Items.Add($item)
}
}
We could leave it there. I think that is acceptable code. If we wanted to however, we could take it further in one of two directions.
First, instead of the five calls to populateComboBox
, we could make a loop from 1 to 5, and get the appropriate properties to pass to the function with Get-ItemProperty
. (I'm assuming that ComboBox1 and so on are really named that way and are not just example names.)
The other idea would be to make an array of (ComboBox, Column) pairs and iterate through that. I quite like that idea because as a general rule it's good to put as much of your code as possible into datastructures. It might be overkill in this case however.
Added stuff:
To expand on the "other idea" above, we can make an array of (ComboBox, Column) pairs. We can use an array of arrays for this:
$comboVals = (
($UI.Combobox1, $ColumnsHashTable.Column1),
($UI.Combobox2, $ColumnsHashTable.Column2),
($UI.Combobox3, $ColumnsHashTable.Column3),
($UI.Combobox4, $ColumnsHashTable.Column4),
($UI.Combobox5, $ColumnsHashTable.Column5)
);
(If you are wondering, I didn't put @
symbols on the arrays here because PowerShell is smart enough to figure out here that we are using arrays.)
We can then iterate through that data structure calling the same populateComboBox
function we defined before:
foreach ($pair in $comboVals)
{
populateComboBox $pair[0] $pair[1]
}
Below is the final script. Note that I haven't tried running this because it is not runnable for me since I don't have the ComboBoxes and columns to test with. I think it should be okay though.
$comboVals = (
($UI.Combobox1, $ColumnsHashTable.Column1),
($UI.Combobox2, $ColumnsHashTable.Column2),
($UI.Combobox3, $ColumnsHashTable.Column3),
($UI.Combobox4, $ColumnsHashTable.Column4),
($UI.Combobox5, $ColumnsHashTable.Column5)
);
foreach ($pair in $comboVals)
{
populateComboBox $pair[0] $pair[1]
}
function populateComboBox($comboBox, $column)
{
foreach ($item in $column)
{
$comboBox.Items.Add($item)
}
}