6
\$\begingroup\$

Background

This is a PowerShell program that uses XAML code from Visual Studio to create a GUI. In this program, there are various Comboboxes (Drop-down menus) used to select different features of the program.

What I'm Trying To Acheieve:

I am trying to populate the Comboboxes with the data stored in the different Hashtables.

The Problem

There are different Comboboxes with different names that need to be populated. I cannot iterate through an array of the different Comboboxes. This is because I cannot (or haven't discovered the solution) have an array of objects. Therefore, this is the solution that I have resulted to. This works and solves my problem. However, I want to make the code more efficient and cleaner looking. I want to hear insight from other programmers as to how they would solve this problem, because I am curious.

The Code

For($i = 0; $i -lt $ColumnsHashTable.Column1.Length; $i++) {

    $UI.Combobox1.Items.Add($ColumnsHashTable.Column1[$i])

}

For($i = 0; $i -lt $ColumnsHashTable.Column2.Length; $i++) {

    $UI.Combobox2.Items.Add($ColumnsHashTable.Column2[$i])

}

For($i = 0; $i -lt $ColumnsHashTable.Column3.Length; $i++) {

    $UI.Combobox3.Items.Add($ColumnsHashTable.Column3[$i])

}

For($i = 0; $i -lt $ColumnsHashTable.Column4.Length; $i++) {

    $UI.Combobox4.Items.Add($ColumnsHashTable.Column4[$i])

}

For($i = 0; $i -lt $ColumnsHashTable.Column5.Length; $i++) {

    $UI.Combobox5.Items.Add($ColumnsHashTable.Column5[$i])

}
\$\endgroup\$
5
\$\begingroup\$

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)
    }
}
\$\endgroup\$
  • \$\begingroup\$ Can you expand upon the "other" idea? I actually had that in my code before. The problem that I was running into, as described in my problem paragraph, is that the array wasn't being populated with the objects $Combobox1, $Combobox2, etc. Whenever I would run just the array, it would return empty. Nevertheless, thank you for your answer. I thought it was intuitive and definitely a better solution that I can use in my future projects. \$\endgroup\$ – Chasen Bettinger Aug 1 '16 at 12:26
  • 1
    \$\begingroup\$ @ChasenBettinger, sure, no problem. See the edits above. If you are getting null values, you could try stepping though your code in the PowerShell ISE. You can examine the variables as you go so you can see where things go wrong. Also put this at the top of all your scripts: Set-StrictMode -Version Latest . That will help catch errors such as spelling mistakes. \$\endgroup\$ – Dangph Aug 1 '16 at 13:48
  • \$\begingroup\$ That is remarkable. Thank you for taking the time to provide a response to my question. \$\endgroup\$ – Chasen Bettinger Aug 1 '16 at 14:49
  • \$\begingroup\$ @ChasenBettinger, you're welcome :) \$\endgroup\$ – Dangph Aug 1 '16 at 14:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.