2015-November Scripting Games Puzzle

Our November 2015 puzzle comes from PowerShell.org user Tim Curwick, who created the puzzle based on a challenge he ran across at work. There's nothing more real-world than this!

Instructions

The Scripting Games have been re-imagined as a monthly puzzle. We publish puzzles the first Saturday of each month, along with solutions and commentary for the previous month's puzzle. You can find them all at https://powershell.org/category/announcements/scripting-games/. Many puzzles will include optional challenges, that you can use to really push your skills.

To participate, add your solution to a public Gist (http://gist.github.com; you'll need a free GitHub account, which all PowerShellers should have anyway). After creating your public Gist, just copy the URL from your browser window and paste it, by itself, as a comment of this post. Only post one entry per person. However, remember that you can always go back and edit your Gist. We'll always pull the most recent one when we display it, so there's no need to post multiple entries if you want to make an edit.

Don't forget the main rules and purpose of these monthly puzzles, including the fact that you won't receive individual scoring or commentary on your entry.

User groups are encouraged to work together on the monthly puzzles. User group leaders should submit their group's best entry to Ed Wilson, the Scripting Guy, via e-mail, prior to the third Saturday of the month. On the last Saturday of the month, Ed will post his favorite, along with commentary and excerpts from noteworthy entries. The user group with the most "favorite" entries of the year will win a grand prize from PowerShell.org.

 

Our Puzzle

Scripting challenge: Understanding and cleaning up someone else's code

Below is an actual script that was in production use at a large enterprise client. The script worked as desired, but as you can see, it could benefit from some clean up. There is some old code in there that may have served a function at one time, but no longer does. The original author and several editors donít seem to have understood PowerShell very well, and it is far more complex than it needs to be.

Your challenge is to replace everything after the Param statement with a single line of code (no semicolons), while retaining all functionality.

We are not looking for the shortest line. The whole point is to make the code more readable. Don't replace unnecessarily complex with unnecessarily cryptic.

As in real life, you should also add internal documentation in the form of any concise comments about the script or your new code which may be of value to the next person troubleshooting or updating the script.

param([string]$VMNameStr)
$VMs=@()
$VMNames=@()
if($VMNameStr.indexof(",") -gt 0)
{
$Trace="Found Comma..."
$VMs=$VMNameStr -split "," | %{$_.trim()}
$trace+="Length = $($VMs.length)"
$trace+=$VMs -is [array]
for($i=0;$i -lt $VMs.length;$i++){
if($VMs[$i] -gt ""){
set-variable -Name ("vmname" + $i) -value $VMs[$i]
$VMNames+=$VMs[$i]
}
}
}
else{$VMName0=$VMNameStr;$VMNames=$VMName0}
$VMNames
Posted in:
About the Author

Don Jones

Don Jones is a Windows PowerShell MVP, author of several Windows PowerShell books (and other IT books), Co-founder and President/CEO of PowerShell.org, PowerShell columnist for Microsoft TechNet Magazine, PowerShell educator, and designer/author of several Windows PowerShell courses (including Microsoft's). Power to the shell!

22 Comments

  1. In order to know what variables the script creates I added "Get-Variable -Include T*,V* -Exclude "VerbosePreference"" to the end of the original script. Then I tested it with one parameter value, test1. The output is shown below.

    PS C:\Temp> .\2015-11-ScriptingGamesPuzzle-Original.ps1 test1
    test1

    Name Value
    ---- -----
    true True
    VMName0 test1
    VMNames test1
    VMNameStr test1
    VMs {}

    Then I tested it with a set of items in the parameter, "Test1, test2,,test3 , test4", which received the following output.

    PS C:\Temp> .\2015-11-ScriptingGamesPuzzle-Original.ps1 "Test1, test2,,test3 , test4"
    Test1
    test2
    test3
    test4

    Name Value
    ---- -----
    Trace Found Comma...Length = 5True
    true True
    vmname0 Test1
    vmname1 test2
    vmname3 test3
    vmname4 test4
    VMNames {Test1, test2, test3, test4}
    VMNameStr Test1, test2,,test3 , test4
    VMs {Test1, test2, , test3...}

    Now I know that the Trace variable is only created if there is more than one item in the parameter set. I also know that the VMs variable is only populated if there is more than one item in the parameter set. Having the VMs variable created without using it is unnecessary and can be eliminated from the code. Also, the Trace variable seems to be tracking what's going on in the script. In my opinion, this is redundant and should be removed. If any tracking needs to occur, Cmdletbinding should be used and verbose messages can be added to the script to see where in the script things are happening. After eliminating the variables Trace and VMs, we are left with VMNameStr, VMNames, and VMName[some integer]. I also know that VMNames output looks like an array and that an items without values were removed, as were any extra spaces.

    Now that I've decided what's essential to the script and what's not. I can start recreating it. But before I do that, there's one more test I should run on the original script. What happens when there is no parameter passed on the command line?

    PS C:\Temp> .\2015-11-ScriptingGamesPuzzle-Original.ps1

    Name Value
    ---- -----
    true True
    VMName0
    VMNames
    VMNameStr
    VMs {}

    The script creates the variables, VMNameStr, VMNames, and VMName[some integer], without populating them with data. To me, this means that the mandatory parameter argument is necessary to ensure that the user inputs a value for $VMNameStr.

    To make the input for $VMNameStr mandatory I added "[parameter(Mandatory=$True)]." Now that I know that the function will always ask for the parameter if none is given, I can move onto parsing the input. The input needs to be split at each comma and trimmed of extra spaces, to create an array of strings. I did this by ($VMNameStr.split(",")).Trim(). Then I needed to eliminate any inputs without text values. I did this using the where-object cmdlet. Now that I've cleaned up VMnames, I just need to create the vmname[some integer] variables. I did this using the Foreach-Object cmdlet and piping $VMnames to it. Within the Foreach-object loop, I created the variable for the integer, "i" in the begin part of the loop, then in the process section I added to "i" before looping to the next section. In the process section I create the vmname[some integer] variables and set it to the value of the item being processed by Foreach-object. The updated code is below.

    https://gist.github.com/Keithlr2/21bfa0b44876d0ee6512#file-gistfile1-txt

  2. param([string]$VMNameStr)
    #This code sets whatever value from the parameter VMNameStr and assigns it to the variable VMNames. If the parameter VMNameStr contains any commas, then the script will replace all the commas with a space. Finally, it will print out to the host.
    $VMNames = $VMNameStr.Replace(',',' ') | Out-Host

    This is my interpretation of what the challenge is this month. Without knowing the full the scope of why the data was formatted the way it was, I have to assume that the results from the original script is what we need to keep it the same (hence why it prints to console the way it does. I don't know the clients requirements of how it needs to be accomplished). I think that this keeps format of the original while making it simple enough to try to reverse engineer.

    Now with that said, if I was to overhaul the operation of the script, I would definitely have the script assign VMNames to an array and make sure that other scripts depending on the output could use it. But I think we all would do some amazing stuff if it was completely up to us 🙂

    • param([string]$VMNameStr)
      $VMNames = $VMNameStr -replace ',',' ' | Out-Host

      Modified it (Don't know how to modify my original comment, new to the site or do the github stuff...yet). But figured to stay away direct .NET methods if the goal is to make this completely user friendly. Also assigned the output as a variable just in case the original script was ran with . in front of it (. .\script.ps1)